Skip to content

Conversation

mvarchdev
Copy link
Contributor

Update the return type of the findExpectedIndex function to include foundIndex and tagLineCount. Adjust the logic in the calling function to utilize the new return structure, ensuring proper handling of expected indices and offsets.

Fixes #1530

@brettz9
Copy link
Collaborator

brettz9 commented Sep 18, 2025

Great to have a fix. Can you also add tests in /test/rules/assertions/require-param.js?

You can test locally, e.g., with pnpm test-index --rule=require-param --invalid=-1

@mvarchdev
Copy link
Contributor Author

@brettz9 i pushed the tests

@brettz9 brettz9 force-pushed the bugfix/#1530-fix-duplicate-parameter-and-malforming-jsdoc branch from bb71eee to ee1c33d Compare September 21, 2025 18:21
@brettz9
Copy link
Collaborator

brettz9 commented Sep 21, 2025

Glad to see this... However, it seems that the test passes even before your changes. Can you add a minimal case which used to fail but no longer does?

@brettz9
Copy link
Collaborator

brettz9 commented Sep 21, 2025

Also, I don't know if your fix can improve this, but as per #540 , we have a couple other require-param issues, with some line numbers deliberately missing from tests because the currently output line numbers would be wrong. It would be great to fix these too if it makes sense for this PR.

@brettz9 brettz9 force-pushed the bugfix/#1530-fix-duplicate-parameter-and-malforming-jsdoc branch 3 times, most recently from 39ba9c8 to e0fac9f Compare September 25, 2025 05:22
@brettz9 brettz9 force-pushed the bugfix/#1530-fix-duplicate-parameter-and-malforming-jsdoc branch from e0fac9f to 08edaf6 Compare September 26, 2025 10:21
@mvarchdev
Copy link
Contributor Author

@brettz9 Hello, sorry, I am a bit time-limited currently as I am moving... so did not see your comments, just now, the thing is that i can very easily show you on quick call or record the video and show what is the problem. On the other stuff, I would really like to look into that, but yeah, firstly I am moving AND I have 2 deadlines coming. So I can look only later. This PR was something we really needed, so we have it in our priv repo as patch and waiting to be merged.

@brettz9
Copy link
Collaborator

brettz9 commented Sep 26, 2025

Sure, no worries. I just added the example you originally gave in #1530 because your other test case seemed to have been fine previously. But I do see #1530 is now fixed. I plan to merge your changes tomorrow if that is ok due to #1519 requesting a greater delay between releases.

@brettz9 brettz9 merged commit 91e261d into gajus:main Sep 27, 2025
4 checks passed
Copy link

🎉 This PR is included in version 60.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@brettz9
Copy link
Collaborator

brettz9 commented Sep 27, 2025

Thanks for the report and fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

require-param fixer inserts new tags inside preceding @example blocks
2 participants