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

StrictUnusedVariable: update severity to a warning, and remove suggested fixes to prefix variables with underscores #2593

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

mpritham
Copy link
Contributor

Before this PR

The StrictUnusedVariable check currently produces errors when a variable is unused unless it is prefixed with an underscore. There are cases (albeit rare) where users legitimately need to keep a reference to an object in scope to prevent the GC from cleaning it. For these scenarios, the underscore prefix (or a suppression) is introduced to satisfy this check.

By moving the severity to the WARNING level, users will still be alerted of their unused variables, and can decide what action they'd like to take.

After this PR

==COMMIT_MSG==
Closes #2361. Change the severity to WARNING. Remove suggested fixes to prefix unused variables with underscores, but keep the suggested fix which renames used variables prefixed with underscores.
==COMMIT_MSG==

Possible downsides?

Pritham Marupaka added 2 commits June 13, 2023 18:10
Refs #2361. Change the severity to `SUGGESTION`. Remove suggested fixes to prefix unused variables with underscores, but keep the suggested fix which renames used variables prefixed with underscores.
@changelog-app
Copy link

changelog-app bot commented Jun 14, 2023

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Closes #2361. Change the severity to WARNING. Remove suggested fixes to prefix unused variables with underscores, but keep the suggested fix which renames used variables prefixed with underscores.

Check the box to generate changelog(s)

  • Generate changelog entry

@@ -133,63 +133,49 @@ void handles_lambdas_in_static_init() {

@Test
public void renames_previous_suppression() {
refactoringTestHelper
.addInputLines(
// if it is prefixed with 'unused' don't do anything.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not the current functionality, but something I was thinking about. It's could be a bit annoying if we show this warning when the user already acknowledged that the variable is unused by prefixing it with unused or ignored.

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.

StrictUnusedVariable check should suggest removing variables assigned in constructor but never read
1 participant