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

Fix RemoveUnusedPatternVars #1345

Merged
merged 3 commits into from
Mar 29, 2021
Merged

Fix RemoveUnusedPatternVars #1345

merged 3 commits into from
Mar 29, 2021

Conversation

vincenzobaz
Copy link
Contributor

@vincenzobaz vincenzobaz commented Mar 2, 2021

This PR aims at fixing #1343 by using a more general regexp and adding several tests.

closes #1343

@vincenzobaz vincenzobaz changed the title Fix #604 Fix #1343 Mar 2, 2021
@vincenzobaz vincenzobaz changed the title Fix #1343 Fix RemoveUnusedPatternVars Mar 2, 2021
@mlachkar
Copy link
Collaborator

mlachkar commented Mar 4, 2021

@vincenzobaz can you check why the tests are failing .. I find that strange the renameSuite is failing. Can you also run bin/scalafmt. Thanks

Base automatically changed from master to main March 25, 2021 13:56
@mlachkar
Copy link
Collaborator

mlachkar commented Mar 29, 2021

I am having troubles understanding the failing tests with this PR. The change should not affect the other tests, but it's the case.
When debugging, the list of diagnostics is empty in the failing tests.
In my tests, the semanticdb files were produce during compilation time, not by interactive.global.
I am quite blocked by this bug. @bjaglin what do you think we should do ?

@mlachkar
Copy link
Collaborator

mlachkar commented Mar 29, 2021

same code, but the tests are passing #1355
The only difference: I removed the newly added file in tests, that make other tests fail

@mlachkar
Copy link
Collaborator

So after further debugging: in fact when adding these new case tests, we don't have warnings anymore on some files...

@mlachkar
Copy link
Collaborator

I have just removed a number of tests that were added by this pull request.
Something in those tests was making the compiler not producing anymore "unused warning" for other files.

@bjaglin
Copy link
Collaborator

bjaglin commented Mar 29, 2021

Something in those tests was making the compiler not producing anymore "unused warning" for other files.

Could it be a limit in the compiler output? https://docs.scala-lang.org/overviews/compiler-options/index.html

-Xmaxwarns ARG

Maximum warnings to print
Default: 100

@mlachkar
Copy link
Collaborator

mlachkar commented Mar 29, 2021

I think it should be that! completly forgot about this :s
Pushing again with the entire file test

@mlachkar
Copy link
Collaborator

mlachkar commented Mar 29, 2021

It also means that this rule fixes maximum 100 warns by default (in each run)^^

@mlachkar mlachkar merged commit 9807b33 into scalacenter:main Mar 29, 2021
@bjaglin
Copy link
Collaborator

bjaglin commented Mar 29, 2021

It also means that this rule fixes maximum 100 warns by default (in each run)^^

#1264

We could mention it in the docs at least?

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.

Remove unused doesn't remove unused pattern vars when inside a method
3 participants