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.

Sign up for GitHub

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

Jump to bottom

[java] Updates to AST nodes #4768

Merged
merged 26 commits into from
Jan 26, 2024
Merged

[java] Updates to AST nodes #4768

merged 26 commits into from
Jan 26, 2024

Conversation

adangel
Copy link
Member

@adangel adangel commented Dec 11, 2023 •
edited

Describe the PR

Finish the remaining open tasks for Java (#1307, #3751).

Note: I don't want to remove any methods for now, just deprecate them, if not already. I think, we should remove the deprecated methods only after a RC5, to have some deprecation notice. However, renaming classes definitely breaks...
Not sure, if it's worth to wait. I'll see, once I proceed with this PR.

In this case, I decided now to remove the method ASTClassDeclaration#isPackagePrivate since we renamed the class from ASTClassOrInterfaceDeclaration. So, deprecating isn't that helpful.

Tasks:

  • push down isStatic to just ASTMethodOrConstructorDeclaration, ASTFieldDeclaration and ASTAnyTypeDeclaration as it's convenient (like isAbstract)
    • Note: moved isStatic to ASTMethodDeclaration instead, as constructors can't be static
  • add a hasVisibility(Visibility) method to AccessNode, to replace the isPublic/Private/*, etc, for the convenience of java rules
  • Remove FinalizableNode, as it's not a solid abstraction, and just push down isFinal to ASTVariableDeclaratorId, ASTAnyTypeDeclaration, ASTFieldDeclaration, and ASTMethodOrConstructorDeclaration for convenience
    • Note: added to ASTAnyTypeDeclaration, ASTCatchParameter, ASTFormalParameter, ASTLambdaParameter, ASTLocalVariableDeclaration, ASTVariableDeclaratorId, ASTMethodDeclaration
  • Rename AccessNode -> ModifierOwner, including API doc in release notes
  • Rename ASTClassOrInterfaceType -> ASTClassType, including API doc in release notes
  • Rename ASTClassOrInterfaceDeclaration -> ASTClassDeclaration, including API doc in release notes
  • Rename ASTAnyTypeDeclaration -> ASTTypeDeclaration (ASTTypeDeclaration is a node in the current tree but was removed), including API doc in release notes
  • Rename ASTMethodOrConstructorDeclaration -> ASTExecutableDeclaration
    • including AbstractMethodOrConstructorDeclaration -> AbstractExecutableDeclaration
  • Rename ASTVariableDeclaratorId -> ASTVariableId
  • Rename ASTClassOrInterfaceBody -> ASTClassBody
  • Fix deprecated API usages in rules (e.g. addViolation calls, deprecated getter usages)
  • Integrate some parts of [java] Remove usages of getImage in java module #4352 - notably:
    • ASTLiteral improvements and introducing getLiteralText(). Note: Using Long.parseUnsignedLong rather than a custom implementation.
    • Expose Chars-typed attributes in XPath, so that @LiteralText is available
    • The goal is, that we don't use deprecated methods anymore, so we can delete them. It's not the goal to improve rule implementations in general.

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)

@adangel
[java] Push isStatic() down to declaration
9869dcc 
Only on ASTMethodDeclaration, ASTFieldDeclaration and ASTAnyTypeDeclaration.
This makes it possible to remove it
from AccessNode.
@adangel
[java] Add hasVisibility method
22e4640 
Deprecate ASTClassOrInterfaceDeclaration#isPackagePrivate().
Use #hasVisibility(Visibility) instead, which can correctly
differentiate between local and package private classes.
@adangel
Fix tests
ae2846d 
* @PackagePrivate is deprecated now and gone
* @Static is no more deprecated for types, methods, fields
@adangel
Fix tests - Use consequently BaseJavaTreeDumpTest
6909a9d
@adangel adangel added the in:ast About the AST structure or API, the parsing step label Dec 11, 2023
@adangel adangel added this to the 7.0.0 milestone Dec 11, 2023
@adangel adangel mentioned this pull request Dec 11, 2023
Closed
55 tasks
@adangel adangel changed the title [jaa] Updates to AST nodes [java] Updates to AST nodes Dec 11, 2023
@pmd-test
Copy link

pmd-test commented Dec 11, 2023 •
edited

1 Message
📖 Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 10 errors and 0 configuration errors.
Download full report as build artifact
✅ Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 10 errors and 0 configuration errors.
Download full report as build artifact
✅ Compared to master:
This changeset changes 0 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 10 errors and 0 configuration errors.
Download full report as build artifact
✅ Compared to master:
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.
Download full report as build artifact
✅ Compared to master:
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.
Download full report as build artifact
✅ Compared to master:
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.
Download full report as build artifact
✅ Compared to master:
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.
Download full report as build artifact
✅ Compared to master:
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.
Download full report as build artifact

Generated by 🚫 Danger

@oowekyala
Copy link
Member

Thanks for starting this! I would also suggest renaming VariableDeclaratorId to VariableId since it's used relatively often in the codebase.

@adangel
Copy link
Member Author

adangel commented Dec 12, 2023

I guess, we should also rename ASTClassOrInterfaceBody to ASTClassBody. It looks odd, if we leave ASTClassOrInterfaceBody... (it's the only one with "ClassOrInterface")

adangel and others added 11 commits December 13, 2023 08:54
@adangel
[java] Rename ASTAnyTypeDeclaration to ASTTypeDeclaration
8b4caed 
Also rename AbstractAnyTypeDeclaration to AbstractTypeDeclaration
@adangel
[java] Rename ASTMethodorConstructorDeclaration to ASTExecutableDecla…
8198218 
…ration

Also rename AbstractMethodOrConstructorDeclaration to AbstractExecutableDeclaration
@adangel
[java] Rename ASTVariableDeclaratorId to ASTVariableId
ba06115
@adangel
[java] Rename ASTClassOrInterfaceBody to ASTClassBody
c579576
@adangel
[java] Update change log in Java.jjt
21ec4b9
@adangel
[java] Avoid usage of deprecated addViolation
e283f11
@adangel
[java] Avoid usage of deprecated methods
0fb4593 
- getName() instead of getMethodName() or getVariableName()
- firstChild() instead of getFirstChildOfType()
- getRoot() instead of getFirstParentOfType(ASTCompilationUnit.class)
- children() instead of findChildrenOfType()
- no more addRuleChainVisit()
- ancestors() instead of getNthParent()
- descendants() instead of findDescendantsOfType()
@adangel
[java] Avoid usage of deprecated methods
7a0f731 
- firstChild() instead of getFirstChildOfType()
- descendants() instead of findDescendantsOfType()
- ancestors() instead of getFirstParentOfType()
@adangel @oowekyala
[core][java] Integrate improvements from pmd#4352 (avoid getImage())
73fcf6e 
- This improves ASTLiteral implementation
- Adds ASTLiteral#getLiteralText() - not yet exposed as XPath attribute

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>
@adangel @oowekyala
[core][java] Expose Chars attributes in XPath
97d141d 
- taken from pmd#4352 (avoid getImage())
- Exposes @LiteralText for ASTLiteral

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>
@adangel
[java] Remove ASTClassDeclaration#isPackagePrivate()
3a3a878
@adangel adangel marked this pull request as ready for review December 14, 2023 14:00
@adangel
Copy link
Member Author

adangel commented Dec 14, 2023 •
edited

I think, this is ready now.
It's unfortunately quite big, but the task list gives a good overview I think.

Many deprecated API usages are resolved. I integrated parts of #4352 (ASTLiteral improvements, XPath Attribute improvements).
I kept out changing getImage in general (which was started in #4352) and only fixed usage, when there is a direct
replacement (e.g. getName). Reasons:

  • I don't want to make this PR even bigger.
  • I think, we should back out the deprecation of Node::getImage and improve it later - see [core] Remove Node::getImage #4318 for that.
    There are still some issues, e.g. XPathRule requires something to get the message for reporting violations...
    without that, we can't finish it.
  • Node::getImage is not deprecated on PMD6, it was only deprecated on the PMD7 branch
  • Similar case is PropertySource::dysfunctionReason - I think, it's better to un-deprecate it and deprecate it
    again only when we have a replacement for it.

Deleting deprecations needs to be done in a separate PR (I don't want to make this even bigger).

Update: Except for release notes :) fixed issues needs to be updated Done

@adangel adangel mentioned this pull request Jan 11, 2024
Merged
4 tasks
oowekyala
oowekyala approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@oowekyala oowekyala left a comment

Choose a reason for hiding this comment

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

Thanks, this looks good

pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/internal/PrettyPrintingUtil.java Outdated Show resolved Hide resolved
adangel and others added 2 commits January 12, 2024 10:30
@adangel @oowekyala
Update pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/inter…
8bd78b4 
…nal/PrettyPrintingUtil.java

Co-authored-by: Clément Fournier <clement.fournier@tu-dresden.de>
@adangel
Merge branch 'master' into java-ast-updates
b17fe92
@adangel adangel merged commit 0c4b4f4 into pmd:master Jan 26, 2024
3 checks passed
@adangel adangel deleted the java-ast-updates branch January 26, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oowekyala oowekyala oowekyala approved these changes

Assignees
No one assigned
Labels
in:ast About the AST structure or API, the parsing step
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

[java] Rename some node types [java] AccessNode API changes
3 participants
@adangel @pmd-test @oowekyala