Skip to content

Add S2209/S3252 recipe: Static members via class name#836

Merged
timtebeek merged 5 commits intomainfrom
tim/static-member-access
Apr 13, 2026
Merged

Add S2209/S3252 recipe: Static members via class name#836
timtebeek merged 5 commits intomainfrom
tim/static-member-access

Conversation

@timtebeek
Copy link
Copy Markdown
Member

Summary

Implements OpenRewrite recipe to fix SonarQube rules S2209 (MAJOR) and S3252 (MINOR). Replaces static field/method access through instance references with access via the declaring class name (e.g., instance.staticFieldClassName.staticField, instance.staticMethod()ClassName.staticMethod()).

Includes JavaIsoVisitor implementation handling field access and method invocations, with conservative side-effect preservation (skips method calls and new expressions as select targets). Full test coverage with 10 test cases.

Implements OpenRewrite recipe to replace static field/method access through instance references with access via the declaring class name. Transforms instance.staticField to ClassName.staticField and instance.staticMethod() to ClassName.staticMethod(). Covers both RSPEC-S2209 (MAJOR) and RSPEC-S3252 rules.
Handle nested class names (e.g. Outer.Inner) by building a J.FieldAccess
chain instead of using getClassName() as a single identifier. Add recursive
side-effect check for chained field access to avoid dropping method calls
in chains like getObj().field.STATIC.
Replace hand-rolled nested class handling with the existing utility,
which correctly handles type attribution, parameterized types, and
local classes.
@timtebeek timtebeek moved this from In Progress to Ready to Review in OpenRewrite Apr 11, 2026
Comment on lines +380 to +396
void doNotChangeChainedFieldAccessWithSideEffect() {
rewriteRun(
//language=java
java(
"""
class MyClass {
static int COUNT = 0;
MyClass field;
static MyClass getInstance() { return new MyClass(); }
void foo() {
int x = getInstance().field.COUNT;
}
}
"""
)
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't the starting state going to cause a null pointer exception? I suppose it doesn't necessarily matter for detection here, but we know we never set field to anything.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Indeed it's a heavily reduced example, but the compiler allows it, and it's enough for detection. Thanks for the ask though!

Copy link
Copy Markdown
Contributor

@steve-aom-elliott steve-aom-elliott left a comment

Choose a reason for hiding this comment

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

I think it makes sense. I suppose to further be able understand whether the method invocation chained into field access would have been safe to swap out with just a field access we would need more detailed analysis of the flow present.

@timtebeek timtebeek merged commit e9d90e7 into main Apr 13, 2026
1 check passed
@timtebeek timtebeek deleted the tim/static-member-access branch April 13, 2026 22:27
@github-project-automation github-project-automation Bot moved this from Ready to Review to Done in OpenRewrite Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants