Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: update check-imports using function #33565

Closed
wants to merge 2 commits into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented May 26, 2020

Currently, the do_exist function that performs the checks for unused
types declared with using declarations, only matches the name of the
using declarations. The means that some unused using declarations are
not detected. For example having both a using Maybe; and a using MaybeLocal and only using MaybeLocal would not be reported.

This commit attempts to take into account the above mentioned case and
also others where the name of the using declaration type is used in name
of a variables or function call. For example, using Just;, will now be
reported as unused even if there are calls to the FromJust function.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Currently, the do_exist function that performs the checks for unused
types declared with using declarations, only matches the name of the
using declarations. The means that some unused using declarations are
not detected. For example having both a 'using Maybe;' and a 'using
MaybeLocal' and only using MaybeLocal would not be reported.

This commit attempts to take into account the above mentioned case and
also others where the name of the using declaration type is used in name
of a variables or function call. For example, 'using Just;', will now be
reported as unused even if there are calls to the FromJust function.
This commit removes the unused using declarations reported by lint-cpp.
@nodejs-github-bot nodejs-github-bot added the tools Issues and PRs related to the tools directory. label May 26, 2020
@richardlau
Copy link
Member

#33268

@danbev
Copy link
Contributor Author

danbev commented May 26, 2020

@richardlau Sorry, for some reason I though that had already been merged. Closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants