[core][most languages] Fix #4814: deprecate AbstractRule#as ctx by UncleOwen · Pull Request #6586 · pmd/pmd

UncleOwen · 2026-04-13T11:14:24Z

Describe the PR

For each language
- Change AbstractFooRule to implement FooVisitor<RuleContext, RuleContext> instead of just FooVisitor.
- Fix rules by replacing Object data with RuleContext data
- Remove calls to AbstractRule.asCtx()
- Remove casts to (RuleContext)
- Some rules in pmd-java had some accidentally public methods deprecated
Some languages didn't need any changes, as they used AbstractVisitorRule, which was already fixed.
Deprecate AbstractRule.asCtx()
Added some language to adding_a_new_javacc_based_language. adding_a_new_antlr_based_language.md recommends using AbstractVisitorRule (see above)

Related issues

Fix [core] Deprecate AbstractRule#asCtx #4814

Ready?

[n/a] 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)

codacy-production · 2026-04-13T11:18:05Z

Not up to standards ⛔

🔴 Issues 10 medium

Alerts:
⚠ 10 issues (≤ 0 issues of at least minor severity)

Results:
10 new issues

Category Results

Complexity 10 medium

View in Codacy

🟢 Metrics 0 complexity · -2 duplication

Metric Results

Complexity 0

Duplication -2

View in Codacy

_{TIP This summary will be updated as you push new changes. Give us feedback}

pmd-actions-helper · 2026-04-13T11:30:26Z

Documentation Preview

Compared to main:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
There are 0 changed duplications, 0 new duplications and 0 removed duplications.
There are 0 changed CPD errors, 0 new CPD errors and 0 removed CPD errors.

Regression Tester Report

(comment created at 2026-04-24 14:46:14+00:00 for 0175d71)

UncleOwen · 2026-04-13T11:37:57Z

I had a look at the codacy report. I get that ApexCRUDViolationRule.checkForAccessibility has a very high NPath complexity. But why is this flagged as new? That was true even before these changes. (Is this maybe the first time codacy scanned this file?)

UncleOwen · 2026-04-24T13:46:01Z

I might do a followup PR that renames all the RuleContext data to RuleContext ctx. But as that would increase the risk of merge conflicts, I'd like to NOT do this here.

… instead of JavaVisitor. Fix all the rules in pmd-java: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx() * Remove casts to (RuleContext) Deprecated some methods that were accidentally public: * FieldDeclarationsShouldBeAtStartOfClassRule.visit(ASTTypeDeclaration, Object) * CyclomaticComplexityRule.visitTypeDecl(ASTTypeDeclaration, Object) * SwitchDensityRule.visitSwitchLike(ASTSwitchLike, Object)

… instead of ApexVisitor. Fix all the rules in pmd-apex: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx() * Remove casts to (RuleContext)

…t> instead of PlsqlVisitor. Fix all the rules in pmd-plsql: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx() * Remove casts to (RuleContext) Deprecated unused method AbstractPLSQLRule.visit(ExecutableCode, Object).

…nstead of VtlVisitor. Fix all the rules in pmd-velocity: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

…tead of VfVisitor. Fix all the rules in pmd-visualforce: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

…nstead of JspVisitor. Fix all the rules in pmd-jsp: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

…Context> instead of ModelicaVisitor. Fix all the rules in pmd-modelica: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

… instead of HtmlVisitor. Fix all the rules in pmd-html: * Use type RuleContext instead of Object * Remove casts to (RuleContext)

…RuleContext> instead of EcmascriptVisitor. Fix all the rules in pmd-javascript: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

adangel

Thanks for starting on this!

I would have expected, that this is a API-incompatible change, but apparently it is not...

Unfortunately we first need to decide, how to fill in the generic types. Using RuleContext as the return type for the visit methods is not useful/does not make sense - and AFAIK we don't use it anywhere...

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, RuleContext> {
public abstract class AbstractJavaRule<R> extends AbstractRule implements JavaVisitor<RuleContext, R> {
public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, ?> {
??

After reading AbstractVisitorRule, we probably should use <RuleContext, ?>.

I might do a followup PR that renames all the RuleContext data to RuleContext ctx. But as that would increase the risk of merge conflicts, I'd like to NOT do this here.

I think, that doesn't make a difference, as you anyway tackle the lines already (to remove the cast or call to asCtx). So better do it directly in one go.

But: This PR is too big to be manageable... You already split the work into nice commits. Please create separate PRs for each language. The deprecation in core can go last.

I had a look at the codacy report. I get that ApexCRUDViolationRule.checkForAccessibility has a very high NPath complexity. But why is this flagged as new? That was true even before these changes. (Is this maybe the first time codacy scanned this file?)

I don't remember anymore why we installed codacy and why it now create PR comments. Apparently we installed in 3 years ago. I assume, they check modified files and analyse these and report anything on these files. "These issues were introduced in your commit, but outside the scope of your changes." - whatever this means...

adangel · 2026-05-07T12:25:42Z

 ### 10. Create an abstract rule class for the language
-*   Extend `AbstractRule` and implement the parser visitor interface for your language *(see AbstractVmRule for example)*
+*   Extend `AbstractRule` and implement the parser visitor interface for your language *(see AbstractVfRule for example)*
+*   Make sure to apply the proper type arguments.


Suggested change

* Make sure to apply the proper type arguments.

* Make sure to apply the proper type arguments for the parser visitor interface: Usually, a `RuleContext` is used as the parameter type and a unbound generic type `R` is used for the return type of each visit method. Through the RuleContext one can directly report violations in the visit methods.

I think, keeping the return type open makes more sense than forcing to use rule context... Most rule I think don't really use the return type and either return null, return super.visit or return data (which is coincidentally the rule context).

adangel · 2026-05-07T12:25:56Z

    behavior for nodes you don’t care about.

 ### 11. Create rules
 *   Rules are created by extending the abstract rule class created in step 9 *(see `EmptyForeachStmtRule` for example)*


Suggested change

* Rules are created by extending the abstract rule class created in step 11 *(see `EmptyForeachStmtRule` for example)*

adangel · 2026-05-07T12:26:25Z


 ### 11. Create rules
 *   Rules are created by extending the abstract rule class created in step 9 *(see `EmptyForeachStmtRule` for example)*
 *   Creating rules is already pretty well documented in PMD - and it’s no different for a new language,


We should better point to the documentation - where is it?
-> https://docs.pmd-code.org/latest/pmd_userdocs_extending_writing_java_rules.html

adangel · 2026-05-07T12:45:19Z

-public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor {
+public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, RuleContext> {

    @Override
-    public Object visitNode(Node node, Object param) {
+    public RuleContext visitNode(Node node, RuleContext param) {
        node.children().forEach(n -> n.acceptVisitor(this, param));
        return param;
    }


To be decided:

Either this:

Suggested change

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor {

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, RuleContext> {

@Override

public Object visitNode(Node node, Object param) {

public RuleContext visitNode(Node node, RuleContext param) {

node.children().forEach(n -> n.acceptVisitor(this, param));

return param;

}

public abstract class AbstractJavaRule<R> extends AbstractRule implements JavaVisitor<RuleContext, R> {

@Override

public R visitNode(Node node, RuleContext param) {

node.children().forEach(n -> n.acceptVisitor(this, param));

return null;

}

Or, as we anyway return null in the end:

Suggested change

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor {

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, RuleContext> {

@Override

public Object visitNode(Node node, Object param) {

public RuleContext visitNode(Node node, RuleContext param) {

node.children().forEach(n -> n.acceptVisitor(this, param));

return param;

}

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, Void> {

@Override

public Void visitNode(Node node, RuleContext param) {

node.children().forEach(n -> n.acceptVisitor(this, param));

return null;

}

Using "Void" has the advantage, that it signals, there is nothing.

UncleOwen · 2026-05-07T13:24:59Z

Now that you mention it... yes, I'm also surprised, that this isn't API-incompatible. Let's make sure this isn't a false-negative in japicmp, I'll investigate.

On the generics question:: If the return value isn't used anywhere, I agree with

public abstract class AbstractJavaRule extends AbstractRule implements JavaVisitor<RuleContext, Void>

I just wasn't sure it wasn't used anywhere.

UncleOwen · 2026-05-07T13:59:58Z

This is definitely API breaking. When I only apply the changes from AbstractJavaRule to the current main (or what was current main before today's merges), all the Java rules stop compiling.

adangel · 2026-05-07T15:13:19Z

This is definitely API breaking. When I only apply the changes from AbstractJavaRule to the current main (or what was current main before today's merges), all the Java rules stop compiling.

Can you try to figure out, why this is not detected by japicmp?

UncleOwen · 2026-05-07T15:43:44Z

This is definitely API breaking. When I only apply the changes from AbstractJavaRule to the current main (or what was current main before today's merges), all the Java rules stop compiling.

Can you try to figure out, why this is not detected by japicmp?

I will look into it.

UncleOwen · 2026-05-07T20:53:50Z

With the updated version of japicmp, this PR fails.

I'll close this now. If I'm bored I might create PRs (one per language) that do as much as we can do without becoming API incompatible, so there is less to do in the future.

adangel · 2026-05-14T10:10:31Z

Thank you @UncleOwen for raising siom79/japicmp#507 !

UncleOwen force-pushed the issue-4814-Deprecate-AbstractRule#asCtx branch from b70a67f to b32a3d4 Compare April 17, 2026 18:28

UncleOwen added 12 commits April 24, 2026 16:16

Make AbstractApexRule implement ApexVisitor<RuleContext, RuleContext>…

5ae18df

… instead of ApexVisitor. Fix all the rules in pmd-apex: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx() * Remove casts to (RuleContext)

Make AbstractVtlRule implement VtlVisitor<RuleContext, RuleContext> i…

33d5e5f

…nstead of VtlVisitor. Fix all the rules in pmd-velocity: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

Make AbstractVfRule implement VfVisitor<RuleContext, RuleContext> ins…

40f1ca0

…tead of VfVisitor. Fix all the rules in pmd-visualforce: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

Make AbstractJspRule implement JspVisitor<RuleContext, RuleContext> i…

8699f3b

…nstead of JspVisitor. Fix all the rules in pmd-jsp: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

Make AbstractModelicaRule implement ModelicaVisitor<RuleContext, Rule…

3229b3a

…Context> instead of ModelicaVisitor. Fix all the rules in pmd-modelica: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

Make AbstractHtmlRule implement HtmlVisitor<RuleContext, RuleContext>…

34ee3e5

… instead of HtmlVisitor. Fix all the rules in pmd-html: * Use type RuleContext instead of Object * Remove casts to (RuleContext)

Make AbstractEcmascriptRule implement EcmascriptVisitor<RuleContext, …

9144a18

…RuleContext> instead of EcmascriptVisitor. Fix all the rules in pmd-javascript: * Use type RuleContext instead of Object * Remove calls to AbstractRule.asCtx()

Deprecate AbstractRule.asCtx

8b14049

Update adding_a_new_javacc_based_language.md

4a81d45

Update writing_java_rules.md

0175d71

UncleOwen force-pushed the issue-4814-Deprecate-AbstractRule#asCtx branch from b32a3d4 to 0175d71 Compare April 24, 2026 14:29

UncleOwen changed the title ~~[core][most languages]Fix #4814: deprecate AbstractRule#as ctx~~ [core][most languages] Fix #4814: deprecate AbstractRule#as ctx Apr 24, 2026

adangel requested changes May 7, 2026

View reviewed changes

adangel added the needs:user-input Maintainers are waiting for feedback from author label May 7, 2026

UncleOwen marked this pull request as draft May 7, 2026 13:26

UncleOwen mentioned this pull request May 7, 2026

False negative involving Generics siom79/japicmp#507

Closed

update japicmp

b2e4891

UncleOwen closed this May 7, 2026

UncleOwen mentioned this pull request May 14, 2026

[java] Part of #4841: Deprecate unnecessary public methods in FieldDeclarationsShouldBeAtStartOfClassRule/CyclomaticComplexityRule/SwitchDensityRule #6671

Merged

2 tasks

	* Make sure to apply the proper type arguments.
	* Make sure to apply the proper type arguments for the parser visitor interface: Usually, a `RuleContext` is used as the parameter type and a unbound generic type `R` is used for the return type of each visit method. Through the RuleContext one can directly report violations in the visit methods.


	* Rules are created by extending the abstract rule class created in step 11 (see `EmptyForeachStmtRule` for example)

Uh oh!

Conversation

UncleOwen commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Related issues

Ready?

Uh oh!

codacy-production Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Not up to standards ⛔

Uh oh!

pmd-actions-helper Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UncleOwen commented Apr 13, 2026

Uh oh!

UncleOwen commented Apr 24, 2026

Uh oh!

adangel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

adangel May 7, 2026

Choose a reason for hiding this comment

Uh oh!

adangel May 7, 2026

Choose a reason for hiding this comment

Uh oh!

UncleOwen May 7, 2026

Choose a reason for hiding this comment

Uh oh!

adangel May 7, 2026

Choose a reason for hiding this comment

Uh oh!

adangel May 7, 2026

Choose a reason for hiding this comment

Uh oh!

UncleOwen commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

UncleOwen commented May 7, 2026

Uh oh!

adangel commented May 7, 2026

Uh oh!

UncleOwen commented May 7, 2026

Uh oh!

UncleOwen commented May 7, 2026

Uh oh!

adangel commented May 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

UncleOwen commented Apr 13, 2026 •

edited

Loading

codacy-production Bot commented Apr 13, 2026 •

edited

Loading

pmd-actions-helper Bot commented Apr 13, 2026 •

edited

Loading

adangel left a comment •

edited

Loading

UncleOwen commented May 7, 2026 •

edited

Loading