Issue #11604: separates detail ast from xpath #11617

rnveach · 2022-05-08T00:29:07Z

This separates DetailAST from xpath so that more of its base classes can be expanded upon and make use of xpath for other ASTs without re-inventing everything. Not much was changed besides splitting the classes in half.

Import control was added to enforce this in the future.

rnveach · 2022-05-08T00:32:43Z

src/main/java/com/puppycrawl/tools/checkstyle/api/CheckstyleAST.java

+package com.puppycrawl.tools.checkstyle.api;
+
+/** Base interface used for all ASTs of Checkstyle. */
+public interface CheckstyleAST {


XPath knows about the underlying AST (see getUnderlyingNode), so to be able to provide this method without just using Object this class was created and all ASTs implement it.

I was originally planning for this to be just AST but PMD complained it was too short. I am open to other names or if this should be named Node. (Are all our ASTs, ASTs, or are they Nodes, or are they....)

Right now it is completely empty, but it will have a future purpose for other common areas that fully rely on the specific ASTs provided. The next target area will be the AstTreeStringPrinter and making the code agnostic that it doesn't care what AST is given to it.

Future planned methods here will be: print (possibly), get line, get column, get type. Some of this when it comes will be breaking changes.

I would prefer to not do Checkstyle,what about AbstractSyntaxTree?

I have no complaints.

rnveach · 2022-05-08T00:34:12Z

CI is expected to fail since it references the issue which has not been approved.

rnveach · 2022-05-15T15:03:32Z

@nick-mancuso Let me know if there is anything else. I renamed the one class.

nrmancuso

Items:

src/main/java/com/puppycrawl/tools/checkstyle/xpath/AbstractElementNode.java

src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractSyntaxTree.java

rnveach · 2022-05-25T00:42:35Z

@romani You have any thoughts on this PR?

romani

Yes, let me block it to think twice on API changes

pbludov

Please fix Javadoc

src/main/java/com/puppycrawl/tools/checkstyle/xpath/AbstractElementNode.java

rnveach · 2022-07-01T01:39:11Z

@romani ping

romani · 2022-09-25T00:12:36Z

Github, rebase

romani · 2022-11-05T01:15:28Z

@rnveach , please rebase

rnveach · 2022-11-05T01:49:01Z

Done.

romani · 2022-11-20T15:46:40Z

@rnveach , please resolve conflict and change commit message to reference PR not a umbrella issue.

romani · 2022-11-20T19:45:13Z

I don't see any bad casting. If no objections, let's squash commits.

rnveach · 2022-11-20T19:59:43Z

Squashed.

I don't see any bad casting

Casting is already in place because the new interface didn't have any methods, so everything needed to be casted to the actual class anyways.

romani

last:

config/import-control.xml

src/main/java/com/puppycrawl/tools/checkstyle/xpath/RootNode.java

romani

Items

config/import-control.xml

romani

ok to merge after squash

rnveach commented May 8, 2022

View reviewed changes

rnveach assigned nrmancuso May 8, 2022

rnveach requested review from nrmancuso and romani May 8, 2022 00:46

rnveach force-pushed the issue_11604_3 branch from 50e7550 to 87f8b3c Compare May 8, 2022 01:46

nrmancuso requested changes May 15, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/xpath/AbstractElementNode.java Show resolved Hide resolved

src/main/java/com/puppycrawl/tools/checkstyle/api/AbstractSyntaxTree.java Outdated Show resolved Hide resolved

rnveach mentioned this pull request May 16, 2022

Remove some hardcoding of specifics so Checkstyle can be more Extensible #11604

Open

rnveach force-pushed the issue_11604_3 branch from 87f8b3c to a383c3f Compare May 16, 2022 15:19

nrmancuso approved these changes May 17, 2022

View reviewed changes

nrmancuso assigned romani and unassigned nrmancuso May 17, 2022

romani requested review from pbludov and strkkk June 6, 2022 00:36

romani requested changes Jun 8, 2022

View reviewed changes

strkkk approved these changes Jun 9, 2022

View reviewed changes

pbludov suggested changes Jun 14, 2022

View reviewed changes

src/main/java/com/puppycrawl/tools/checkstyle/xpath/AbstractElementNode.java Outdated Show resolved Hide resolved

rnveach force-pushed the issue_11604_3 branch from a383c3f to 83055bd Compare June 14, 2022 16:58

pbludov approved these changes Jun 18, 2022

View reviewed changes

rnveach force-pushed the issue_11604_3 branch from 83055bd to 77ab6f3 Compare July 1, 2022 01:11

rnveach force-pushed the issue_11604_3 branch 2 times, most recently from 4631083 to 4b6cc86 Compare August 4, 2022 01:38

rnveach force-pushed the issue_11604_3 branch 3 times, most recently from a0c22d9 to 7a13a77 Compare August 30, 2022 00:49

rnveach force-pushed the issue_11604_3 branch from 7a13a77 to db6b7ee Compare September 25, 2022 00:27

rnveach force-pushed the issue_11604_3 branch from 40a3abb to 6011777 Compare November 5, 2022 01:48

rnveach force-pushed the issue_11604_3 branch from 6011777 to 990544d Compare November 5, 2022 02:39

rnveach added a commit to rnveach/checkstyle that referenced this pull request Nov 20, 2022

Pull checkstyle#11617: separates detail ast from xpath

d714807

rnveach force-pushed the issue_11604_3 branch from 990544d to d714807 Compare November 20, 2022 16:04

rnveach added a commit to rnveach/checkstyle that referenced this pull request Nov 20, 2022

Pull checkstyle#11617: separates detail ast from xpath

1bedb43

rnveach force-pushed the issue_11604_3 branch from d714807 to c692c68 Compare November 20, 2022 16:45

rnveach added a commit to rnveach/checkstyle that referenced this pull request Nov 20, 2022

Pull checkstyle#11617: separates detail ast from xpath

09b02aa

rnveach force-pushed the issue_11604_3 branch from c692c68 to 09b02aa Compare November 20, 2022 19:58

This comment was marked as outdated.

Sign in to view

romani requested changes Nov 23, 2022

View reviewed changes

config/import-control.xml Outdated Show resolved Hide resolved

src/main/java/com/puppycrawl/tools/checkstyle/xpath/RootNode.java Outdated Show resolved Hide resolved

rnveach added a commit to rnveach/checkstyle that referenced this pull request Nov 23, 2022

Pull checkstyle#11617: separates detail ast from xpath

0deaaa2

rnveach force-pushed the issue_11604_3 branch from 09b02aa to 0deaaa2 Compare November 23, 2022 15:13

romani requested changes Nov 23, 2022

View reviewed changes

config/import-control.xml Show resolved Hide resolved

rnveach added a commit to rnveach/checkstyle that referenced this pull request Nov 24, 2022

Pull checkstyle#11617: separates detail ast from xpath

f06eacc

rnveach force-pushed the issue_11604_3 branch from 0deaaa2 to f06eacc Compare November 24, 2022 01:43

romani approved these changes Nov 24, 2022

View reviewed changes

romani assigned rnveach and unassigned romani Nov 24, 2022

romani added approved miscellaneous labels Nov 24, 2022

Pull checkstyle#11617: separates detail ast from xpath

0b26308

rnveach force-pushed the issue_11604_3 branch from ab9a45b to 0b26308 Compare November 24, 2022 16:31

romani merged commit 5e413ff into checkstyle:master Nov 24, 2022

rnveach deleted the issue_11604_3 branch November 24, 2022 17:49

rnveach mentioned this pull request Dec 8, 2022

New Check: XMLMatchXPath rnveach/checkstyle-extras#7

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #11604: separates detail ast from xpath #11617

Issue #11604: separates detail ast from xpath #11617

rnveach commented May 8, 2022 •

edited

rnveach May 8, 2022 •

edited

nrmancuso May 8, 2022 •

edited

rnveach May 8, 2022

rnveach commented May 8, 2022

rnveach commented May 15, 2022

nrmancuso left a comment

rnveach commented May 25, 2022

romani left a comment

pbludov left a comment

rnveach commented Jul 1, 2022

romani commented Sep 25, 2022

romani commented Nov 5, 2022

rnveach commented Nov 5, 2022

romani commented Nov 20, 2022

romani commented Nov 20, 2022

rnveach commented Nov 20, 2022

This comment was marked as outdated.

romani left a comment

romani left a comment

romani left a comment

Issue #11604: separates detail ast from xpath #11617

Issue #11604: separates detail ast from xpath #11617

Conversation

rnveach commented May 8, 2022 • edited

rnveach May 8, 2022 • edited

Choose a reason for hiding this comment

nrmancuso May 8, 2022 • edited

Choose a reason for hiding this comment

rnveach May 8, 2022

Choose a reason for hiding this comment

rnveach commented May 8, 2022

rnveach commented May 15, 2022

nrmancuso left a comment

Choose a reason for hiding this comment

rnveach commented May 25, 2022

romani left a comment

Choose a reason for hiding this comment

pbludov left a comment

Choose a reason for hiding this comment

rnveach commented Jul 1, 2022

romani commented Sep 25, 2022

romani commented Nov 5, 2022

rnveach commented Nov 5, 2022

romani commented Nov 20, 2022

romani commented Nov 20, 2022

rnveach commented Nov 20, 2022

This comment was marked as outdated.

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

romani left a comment

Choose a reason for hiding this comment

rnveach commented May 8, 2022 •

edited

rnveach May 8, 2022 •

edited

nrmancuso May 8, 2022 •

edited