Skip to content

Fix #5770: [java] UnnecessaryLocalBeforeBranchRule#5835

Closed
Pankraz76 wants to merge 1 commit intopmd:mainfrom
Pankraz76:UnnecessaryLocalBeforeThrow
Closed

Fix #5770: [java] UnnecessaryLocalBeforeBranchRule#5835
Pankraz76 wants to merge 1 commit intopmd:mainfrom
Pankraz76:UnnecessaryLocalBeforeThrow

Conversation

@Pankraz76
Copy link
Copy Markdown
Contributor

@Pankraz76 Pankraz76 commented Jun 19, 2025 •
edited
Loading

Fix #5770: [java] UnnecessaryLocalBeforeBranchRule

Add derivate of UnnecessaryLocalBeforeReturnRule to cover throw statement as well, considering UnnecessaryLocalBeforeThrowRule.

Related issues

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by github actions)
  • Added (in-code) documentation (if needed)

@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 19, 2025 •
edited
Loading

need to fix imports...

[ERROR] src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeThrowRule.java:[15,1] (imports) ImportOrder: Wrong order for 'net.sourceforge.pmd.properties.PropertyFactory.booleanProperty' import.

@Pankraz76 Pankraz76 changed the title Unnecessary local before throw [java] UnnecessaryLocalBeforeThrowRule Jun 19, 2025
@Pankraz76 Pankraz76 mentioned this pull request Jun 19, 2025
Closed
@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from ec64999 to 07e2497 Compare June 19, 2025 20:27
@Pankraz76 Pankraz76 marked this pull request as ready for review June 19, 2025 20:28
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 19, 2025 •
edited
Loading

Documentation Preview

Compared to main:
This changeset changes 0 violations,
introduces 8 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.

Regression Tester Report

(comment created at 2025-06-22 21:12:49+00:00 for 5bbfebd)

@Pankraz76 Pankraz76 changed the title [java] UnnecessaryLocalBeforeThrowRule [java] UnnecessaryLocalBeforeBranchRule Jun 20, 2025
@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from 07e2497 to 9663264 Compare June 20, 2025 11:23
@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 20, 2025

item:

pmd-java: Versions of archives indicate a minor change but binary incompatible changes found.

how to fix this please? assuming its related to new version since="7.15.0".

@oowekyala

Pankraz76
Pankraz76 commented Jun 20, 2025
View reviewed changes
Comment thread pmd-java/src/main/resources/category/java/codestyle.xml Outdated
@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 20, 2025 •
edited
Loading

should we give rewrite migration script for this, or does the license impose issues here as well?

im sure @iddeepak can grasp this very fast, like always, provide PoC.

@iddeepak
Copy link
Copy Markdown

iddeepak commented Jun 20, 2025

should we give rewrite migration script for this, or does the license impose issues here as well?

im sure @iddeepak can grasp this very fast, like always, provide PoC.

Thanks @Pankraz76! I believe it depends on the implementation, since Rewrite is under the Moderne license.

adangel
adangel requested changes Jun 22, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@adangel adangel left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! See my comments.

Also note: I think, your PR is fixing #5770. If so, please add this in the PR description with "* Fix #...", so that github can link the PR to the issue.

I think, #5776 can be considered totally independent of this PR - it is not a prerequisite. It is maybe something you stumbled upon while working on it, but it is a different topic.

Also in general: The description of the PR is empty... by looking at the changed code, I can re-engineer what you have done, but it would be nice, if you summarize and describe what you do.

Comment thread docs/pages/pmd/projectdocs/credits.md Outdated
Comment thread docs/pages/pmd/projectdocs/credits.md Outdated
Comment thread docs/pages/pmd/projectdocs/credits.md Outdated
Comment thread docs/pages/release_notes_old.md
Comment thread docs/pages/release_notes_pmd7.md Outdated
Comment thread pmd-java/src/main/resources/category/java/codestyle.xml Outdated
Comment thread .../resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeBranch.xml Outdated
Comment thread .../resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeBranch.xml Outdated
Comment thread .../resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeBranch.xml Outdated
Comment thread .../resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeBranch.xml Outdated
oowekyala
oowekyala reviewed Jun 22, 2025
View reviewed changes
Comment thread pmd-java/src/main/resources/category/java/codestyle.xml Outdated
Comment thread pmd-java/src/main/resources/category/java/codestyle.xml Outdated
Comment thread .../resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeBranch.xml Outdated
@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from 6f63604 to c3f4b15 Compare June 22, 2025 19:54
@Pankraz76 Pankraz76 changed the title [java] UnnecessaryLocalBeforeBranchRule Fix #5770: [java] UnnecessaryLocalBeforeBranchRule Jun 22, 2025
@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from c3f4b15 to 5cb8567 Compare June 22, 2025 20:28
@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 22, 2025

I think, #5776 can be considered totally independent of this PR - it is not a prerequisite. It is maybe something you stumbled upon while working on it, but it is a different topic.

yes sorry, reverted already. Had troubled fixing imports, so used proper config.

Thanks.

@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from 5cb8567 to 90a3eeb Compare June 22, 2025 20:29
@Pankraz76 Pankraz76 requested review from adangel and oowekyala June 22, 2025 20:30
Pankraz76
Pankraz76 commented Jun 22, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@Pankraz76 Pankraz76 left a comment

Choose a reason for hiding this comment

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

flaws to be fixed by spot.

Comment thread pmd-java/src/main/resources/category/java/codestyle.xml
Comment thread .../main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeThrowRule.java
Comment thread .../main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeThrowRule.java
@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from 90a3eeb to eaa109c Compare June 22, 2025 20:42
@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 22, 2025

item:

Error: src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeThrowRule.java:[1] (header) RegexpHeader: Line does not match expected header line of '^/**?[ ]*$'.

header will be fixed by spot too. This PR shows it will help everyone in the project to save time.

@Pankraz76 Pankraz76 force-pushed the UnnecessaryLocalBeforeThrow branch from eaa109c to 5bbfebd Compare June 22, 2025 20:51
@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 23, 2025 •
edited
Loading

should be close this or push to here the deprecated version as well?

Deprecating new code makes not sense, therefore should migrate, as not breaking anymore.

Having it generic by simply Removing the relation to thats its before, or adding the name instead Branch or Exit needs to be sorted out. How you want to name the new consolidated rule?

  • UnnecessaryLocalBeforeRule
  • UnnecessaryLocalBeforeBranchRule
  • UnnecessaryLocalBeforeExitRule
  • UnnecessaryLocalBeforeEndRule
  • UnnecessaryDeclarationRule
    • ? Is it maybe a subelement of it?

This could fit as well, as its about declaration of a redundant local field.
Local could be a method as well.

@Pankraz76
Copy link
Copy Markdown
Contributor Author

Pankraz76 commented Jun 28, 2025

continue here:

as this one is missing adjustment of : quickstart.xml

@Pankraz76 Pankraz76 closed this Jun 28, 2025
@adangel adangel mentioned this pull request Jun 29, 2025
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel Awaiting requested review from adangel

@oowekyala oowekyala Awaiting requested review from oowekyala

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[java] New Rule: VariableCanBeInlined: Local variables should not be declared and then immediately returned or thrown

4 participants

@Pankraz76 @iddeepak @adangel @oowekyala