Skip to content

[core] Remove processing stages#3810

Merged
oowekyala merged 18 commits into
pmd:pmd/7.0.xfrom
oowekyala:remove-processing-stages
Mar 5, 2022
Merged

[core] Remove processing stages#3810
oowekyala merged 18 commits into
pmd:pmd/7.0.xfrom
oowekyala:remove-processing-stages

Conversation

@oowekyala

@oowekyala oowekyala commented Feb 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Describe the PR

Remove support for the processing stages introduced by #1426. These stages are clunky and PMD 7 evolved in such a way that we don't need their features (namely, that these passes are optional and only run if a rule declares a dependency on it). The initial use case (way back in PMD 3) was to make type resolution optional while it was experimental and slow. The new Java type resolution of PMD 7 is neither, and most rules depend on it anyway, so it doesn't make sense to turn it off.

All processing stages are now non-optional in PMD 7. This concerns the Java, Modelica and PL/SQL modules. If there is a need to disable some stages at some point, it could be done by a language property #2518. The Java parser still supports turning off attribution passes, solely to test the parser output before disambiguation occurs (although I found out that no tests currently require it... ie there are some tests missing).

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)

@oowekyala oowekyala added this to the 7.0.0 milestone Feb 25, 2022
adangel
adangel approved these changes Feb 26, 2022
View reviewed changes

@adangel adangel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

Comment thread pmd-core/src/main/java/net/sourceforge/pmd/benchmark/TimeTracker.java Outdated
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/benchmark/TimeTracker.java Outdated
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Parser.java Outdated
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/Parser.java
Comment thread pmd-java/src/main/java/net/sourceforge/pmd/lang/java/internal/JavaAstProcessor.java Outdated
Comment thread pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTSwitchStatementTest.java
Comment thread pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/BaseParserTest.java
Comment thread pmd-plsql/src/main/java/net/sourceforge/pmd/lang/plsql/symboltable/SymbolFacade.java

public class SymbolFacade {
public void initializeWith(ASTInput node) {
public final class SymbolFacade {

@adangel adangel Feb 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we mark that as well @InternalApi?

@oowekyala oowekyala Feb 27, 2022

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.

I think so, but many other classes in this package should be internalized. The base symbol table classes should also be moved out of pmd-core into pmd PLSQL I think

@ghost

ghost commented Feb 26, 2022 •
edited by ghost
Loading

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 26 violations,
introduces 20 new violations, 0 new errors and 0 new configuration errors,
removes 20 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 57348 violations,
introduces 34623 new violations, 7 new errors and 0 new configuration errors,
removes 138168 violations, 25 errors and 3 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 246 violations,
introduces 9131 new violations, 36 new errors and 0 new configuration errors,
removes 13840 violations, 6 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 53001 violations,
introduces 38209 new violations, 37 new errors and 0 new configuration errors,
removes 146454 violations, 25 errors and 3 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 241 violations,
introduces 9130 new violations, 36 new errors and 0 new configuration errors,
removes 13839 violations, 6 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 53000 violations,
introduces 38210 new violations, 37 new errors and 0 new configuration errors,
removes 146455 violations, 25 errors and 3 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 239 violations,
introduces 9122 new violations, 36 new errors and 0 new configuration errors,
removes 13838 violations, 6 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 53004 violations,
introduces 38199 new violations, 37 new errors and 0 new configuration errors,
removes 146451 violations, 25 errors and 3 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala
Use AssertionUtil
e4547df
@oowekyala
Cleanup
e6e8488
@oowekyala
Report semantic errors by default
ee0e751 
The dogfood failure is probably caused by missing
classes on the classpath, as I can't reproduce it
locally with a full classpath, but can with an
incomplete one.
@oowekyala oowekyala force-pushed the remove-processing-stages branch from f972083 to ee0e751 Compare February 27, 2022 13:47
@oowekyala

oowekyala commented Feb 27, 2022

Copy link
Copy Markdown
Member Author

@adangel The dogfood failure is caused by unresolved types in pmd-apex. It seems like the compiled classes of pmd apex cannot be resolved (I enabled logging of unresolved types in ee0e751):

https://github.com/pmd/pmd/runs/5350631098?check_suite_focus=true#step:7:11118

@adangel

adangel commented Mar 1, 2022

Copy link
Copy Markdown
Member

@adangel The dogfood failure is caused by unresolved types in pmd-apex. It seems like the compiled classes of pmd apex cannot be resolved (I enabled logging of unresolved types in ee0e751):

https://github.com/pmd/pmd/runs/5350631098?check_suite_focus=true#step:7:11118

Do we have a problem, how maven-pmd-plugin provides the classpath to PMD 7? The compiled classes should be there (target/classes).

Nice logging, btw :)

I think, I found the problem - PmdRunnable creates a ParserTask without the auxclasspath...

adangel
adangel reviewed Mar 1, 2022
View reviewed changes
Comment thread pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java Outdated
@oowekyala oowekyala merged commit 7d36dda into pmd:pmd/7.0.x Mar 5, 2022
@oowekyala oowekyala deleted the remove-processing-stages branch March 8, 2022 20:48
adangel added a commit to pmd/pmd-designer that referenced this pull request Apr 12, 2022
@adangel @oowekyala
Remove processing stages
7bf3fb9 
See pmd/pmd#3810
Taken from #51

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>
@adangel adangel mentioned this pull request Jan 23, 2023
Closed
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adangel adangel adangel approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@oowekyala @adangel