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

Improve -Wunused: locals, privates with unset vars warning #16639 #17160

Merged
merged 2 commits into from
May 16, 2023

Conversation

schuetzcarl
Copy link
Contributor

This PR is related to my Bachelor Semester Project, supervised by @anatoliykmetyuk.

The latter consist in improving and implementing more Scala 3 linter options (see #15503), with #16639 as a starting issue fixed in this PR.

  • During the traversal in CheckUnused.scala (Miniphase & local TreeTraverser), when reaching an Assign case, symbols are collected as set, and then used to filter used locals and privates variable at reporting time.
  • Adapt test suit, and Add more test accordingly.
  • Note that for a same variable the unused warning always has priority and shadows the unset warning.

That feature follows the Scala 2 -Ywarn-unused:<args> behavior.

@szymon-rd
Copy link
Contributor

szymon-rd commented Apr 24, 2023

I was sick the previous week 🤒 So sorry for the delay in reviewing @schuetzcarl But could you please rebase and fix conflicts in this PR? A lot changed in the CheckUnused when we were fixing the bugs for the new RC for 3.3.0

@schuetzcarl schuetzcarl force-pushed the implement/Wunused_is_var_set branch from 8713944 to 9facd95 Compare April 26, 2023 23:24
@schuetzcarl schuetzcarl force-pushed the implement/Wunused_is_var_set branch from 9facd95 to d4bd097 Compare April 26, 2023 23:52
@schuetzcarl
Copy link
Contributor Author

I was sick the previous week face_with_thermometer So sorry for the delay in reviewing @schuetzcarl But could you please rebase and fix conflicts in this PR? A lot changed in the CheckUnused when we were fixing the bugs for the new RC for 3.3.0

Hi Szymon,

No problem, I hope you are fine now.
As requested, I resolved the conflicts and rebased the PR.

Copy link
Contributor

@szymon-rd szymon-rd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've gone through it and almost everything seems great, I have only one comment.

Nil
(Nil, Nil)
val sortedPrivateDefs = unusedPrivates.filterNot(d => containsSyntheticSuffix(d.symbol)).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.PrivateMembers)).toList
val unsetPrivateDefs = unsetVarsFromUsedSym(usedPrivates).map(d => UnusedSymbol(d.namePos, d.name, WarnTypes.UnsetPrivates)).toList
Copy link
Contributor

@szymon-rd szymon-rd Apr 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the function unsetVarsFromUsedSym to a boolean-returning isUnsetVar and using it with .filter would be easier to read, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review :)
I changed the unsetVarsFromUsedSym to an isUnsetVarDef helper function in the tpd.MemberDef extension object.

Let me know if it is convenient for you.

@szymon-rd szymon-rd merged commit 937b91c into scala:main May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve -Wunused:locals,privates by tracking use of vars (if set or not)
3 participants