[java] New Java XPath functions by oowekyala · Pull Request #2917 · pmd/pmd

oowekyala · 2020-11-13T13:58:14Z

Describe the PR

These are changes extracted from #2899 for ease of reference, namely:

small changes to pmd-core and pmd-lang-test
- I just improved error reporting for XPath rules. Now a failure should give more info, also, all failures are reported as PmdXPathException, including failure in our custom functions (currently there's a mix of raw RuntimeException, IllegalArgumentException, IllegalStateException)
the changes to parts of pmd-java that are not rule specific, including
- minute improvements to the ast api
- bugfixes for TypeTestUtil, based on the regression tester feedback from [java] Update many java rules #2899
- the new InvocationMatcher type, which matches method/constructor calls based on a string signature. There is a corresponding XPath function: matchesSig
- Other new XPath functions:
  - nodeIs: refs [core] Match abstract types in XPath queries #1787
  - hasAnnotation: refs [core] Match abstract types in XPath queries #1787
  - modifiers, explicitModifiers: refs [java] AccessNode API changes #1307

This should be merged before #2899, to reduce that PR's diff

Related issues

Addresses [core] Match abstract types in XPath queries #1787 for java
Refs [java] Java-grammar progress document #2701
Part of [core] XPath Improvements for PMD 7 #2523

Ready?

Added unit tests for fixed bug/feature
Passing all unit tests
Complete build ./mvnw clean verify passes (checked automatically by travis)
Added (in-code) documentation (if needed)

ghost · 2020-11-15T15:23:07Z

	2 Messages
📖	Compared to pmd/7.0.x: This changeset changes 0 violations, introduces 20 new violations, 1 new errors and 0 new configuration errors, removes 0 violations, 2 errors and 0 configuration errors. Full report
📖	Compared to master: This changeset changes 695 violations, introduces 1265 new violations, 10 new errors and 0 new configuration errors, removes 1059 violations, 16 errors and 2 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset changes 0 violations, introduces 20 new violations, 10 new errors and 0 new configuration errors, removes 0 violations, 11 errors and 0 configuration errors. Full report
✅	Compared to master: This changeset changes 695 violations, introduces 1258 new violations, 10 new errors and 0 new configuration errors, removes 1059 violations, 16 errors and 2 configuration errors. Full report
✅	Compared to pmd/7.0.x: This changeset introduces new violations, {:errors=>{:new=>10, :removed=>11}, :violations=>{:new=>20, :removed=>0, :changed=>0}, :configerrors=>{:new=>0, :removed=>0}} new errors and new configuration errors, removes violations, errors and configuration errors. Full report
✅	Compared to master: This changeset introduces new violations, {:errors=>{:new=>10, :removed=>16}, :violations=>{:new=>1258, :removed=>1059, :changed=>695}, :configerrors=>{:new=>0, :removed=>2}} new errors and new configuration errors, removes violations, errors and configuration errors. Full report

Generated by 🚫 Danger

adangel · 2020-11-23T13:50:28Z

E, [2020-11-15T15:23:06.773764 #11856] ERROR -- : Running pmdtester failed: #<Slop::MissingArgument: missing argument for -b, --base-branch>

Maybe the regression tester is not working yet for PRs against pmd/7.0.x? I'm still working through mails...

adangel · 2020-11-24T08:16:19Z

Ok, re-running the workflow is only useful for temporary failed builds. It will re-run the builds with exactly the same git ref/sha, see https://docs.github.com/en/free-pro-team@latest/actions/managing-workflow-runs/re-running-a-workflow:

Re-running a workflow uses the same GITHUB_SHA (commit SHA) and GITHUB_REF (Git ref) of the original event that triggered the workflow run.

If the base branch of a PR as advanced, that's not taken into consideration. The only way to rebuild seems to be a real push to the PR branch. Then a new pull_request event is created with an update git ref/sha.

I've pushed another merge now, let's see, if the regression tester is running now correctly...

Btw: the github ref/sha in pull requests is always the test merge commit created automatically by github.

adangel · 2020-11-24T08:50:20Z

Isn't it fun? New error:

   Please update your dependency to directly use the correct version 'xml-apis:xml-apis:1.0.b2'.
  Resolution will only pick dependencies of the relocated element.  Artifacts and other metadata will be ignored.
  
  E, [2020-11-24T08:28:21.876609 #12106] ERROR -- : POM relocation to an other version number is not fully supported in Gradle : xml-apis:xml-apis:2.0.2 relocated to xml-apis:xml-apis:1.0.b2.
  
  FAILURE: Build failed with an exception.
  
  * What went wrong:
  A problem occurred configuring root project 'spring'.
  > Could not resolve all files for configuration ':classpath'.
     > Could not resolve com.beust:jcommander:1.35.
       Required by:
           project : > org.asciidoctor:asciidoctorj-pdf:1.5.0-alpha.16 > org.asciidoctor:asciidoctorj:1.5.6
        > Could not resolve com.beust:jcommander:1.35.
           > Could not get resource 'https://plugins.gradle.org/m2/com/beust/jcommander/1.35/jcommander-1.35.pom'.
              > Could not GET 'https://plugins.gradle.org/m2/com/beust/jcommander/1.35/jcommander-1.35.pom'.
                 > Read timed out

I'm wondering, why this didn't appear in other builds yet?
Looks like the same error in Azure clouds and incompatible TTL settings for pooled HTTP connections....

adangel · 2020-11-24T09:20:31Z

Rerun, now back at sourceforge failure:

  --2020-11-24 09:09:44--  https://master.dl.sourceforge.net/project/pmd/pmd-regression-tester/pmd_7.0.x-baseline.zip?viasf=1
  Resolving master.dl.sourceforge.net (master.dl.sourceforge.net)... 216.105.38.12
  Connecting to master.dl.sourceforge.net (master.dl.sourceforge.net)|216.105.38.12|:443... connected.
  HTTP request sent, awaiting response... 503 Service Unavailable
  2020-11-24 09:10:00 ERROR 503: Service Unavailable.

oowekyala · 2020-11-25T11:36:41Z

https://github.com/pmd/pmd/pull/2917/checks?check_run_id=1453031344#step:6:43193

Running pmdtester failed: #<NoMethodError: undefined method `download_baseline' for #Danger::Dangerfile:0x00005586ba34e8d8>

Looks like it comes from this line (or L44). Are these calls really needed? It looks like the baseline would be downloaded within the Runner::run call anyway

adangel

Running pmdtester failed: #<NoMethodError: undefined method `download_baseline' for #Danger::Dangerfile:0x00005586ba34e8d8>

Looks like it comes from this line (or L44). Are these calls really needed? It looks like the baseline would be downloaded within the Runner::run call anyway

I've fixed that already, probably forgot to delete these lines when merging (ce99966)

I've pushed a small fixes commit to this PR, that should trigger the rebuild. The build of a PR happens always on a test merge commit that github automatically creates, so it should include the fix on pmd/7.0.x.

If then GH actions is happy and regression tester runs, I'll merge this PR.

adangel · 2020-11-24T08:32:06Z

+     * qname       ::= java binary name
+     * }</pre>
+     */
+    public static final class InvocationMatcher {


It might be better, to have the class InvocationMatcher in a separate file instead in TypeTestUtil.... something we can consider later, though. It's new API, so we can still move it around.

adangel · 2020-11-24T08:36:48Z

+    @Override
+    protected Class<?> parseArgument(String constantArg) throws XPathException {
+        try {
+            return Class.forName("net.sourceforge.pmd.lang.java.ast.AST" + constantArg);


This provides the "nodeIs" function for java. The issue #1787 describes this actually as a general, core function, available for all languages. I think, it would be not too hard to port this class + BaseRewrittenFunction to core. But IMHO we should wait with that for later.
Therefore I'll keep #1787 open, as it is only fixed for java.

adangel · 2020-11-25T15:42:27Z

introduces 20 new violations
https://chunk.io/pmd/9325997603b34530a06bee6cd769917c/diff1/spring-framework/index.html

I wonder, where these come from. These are all comment rules, that have been updated with #2802 ...

adangel · 2020-11-25T15:44:30Z

introduces 20 new violations
https://chunk.io/pmd/9325997603b34530a06bee6cd769917c/diff1/spring-framework/index.html

I wonder, where these come from. These are all comment rules, that have been updated with #2802 ...

Ah... it's one specific file: ReactorNettyTcpStompClientTests.java - previously, we had a parser error (java.lang.IllegalStateException: Disambiguation pass should resolve everything except qualified ctor calls) which is now solved...

oowekyala · 2020-11-25T16:08:51Z

Ah... it's one specific file: ReactorNettyTcpStompClientTests.java - previously, we had a parser error (java.lang.IllegalStateException: Disambiguation pass should resolve everything except qualified ctor calls) which is now solved...

I was thinking that we could add some detection of this into pmd/pmd-regression-tester. We already detect "changed" violations. There could be "lost" violations, those that were deleted because of an error, and "recovered" violations, those that appeared because some error disappeared

* Improve docs * Add missing tests for XPath functions * TypeIsFunction has been replaced with BaseContextNodeTestFun, so this deletes the file getCommentOn is still untested. I wonder whether we really need it, also, it's inefficient and not specified at all (because it's unused).

oowekyala added 7 commits November 13, 2020 14:29

Extract changes to xpath functions

150f8c7

Extract changes to invocation matchers

c35b4e8

Extract xpath fun doc

4b98c40

Extract changes to ast

fbc1256

Extract changes to core

715c8fa

Add location information

f287f56

Extract changes to lang-test

67d3b8a

oowekyala added an:enhancement An improvement on existing features / rules in:xpath Relating to xpath support at large, eg Jaxen / Saxon, custom functions, attribute resolution labels Nov 13, 2020

oowekyala added this to the 7.0.0 milestone Nov 13, 2020

This was referenced Nov 13, 2020

[java] Update many java rules #2899

Merged

[java] AccessNode API changes #1307

Closed

oowekyala added 3 commits November 13, 2020 15:15

Fix documentation

2b10c79

Fix exception not thrown

7635a19

Merge branch '7.0.x' into java-new-xpath-functions

bc15bb9

adangel self-assigned this Nov 23, 2020

adangel mentioned this pull request Nov 23, 2020

[core] XPath Improvements for PMD 7 #2523

Closed

14 tasks

Merge branch 'pmd/7.0.x' into pr-2917

3651e66

Merge branch '7.0.x' into java-new-xpath-functions

f8ed149

Small fixes (since=7.0.0, unused ExpectedExeption)

9d51879

adangel reviewed Nov 25, 2020

View reviewed changes

adangel mentioned this pull request Nov 25, 2020

[core] Match abstract types in XPath queries #1787

Open

adangel merged commit 915163d into pmd:pmd/7.0.x Nov 25, 2020

oowekyala deleted the java-new-xpath-functions branch November 25, 2020 16:08

adangel mentioned this pull request Jan 23, 2023

PMD 7 Tracking Issue #3898

Closed

55 tasks

Uh oh!

Conversation

oowekyala commented Nov 13, 2020 • edited by adangel Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe the PR

Related issues

Ready?

Uh oh!

ghost commented Nov 15, 2020 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adangel commented Nov 23, 2020

Uh oh!

adangel commented Nov 24, 2020

Uh oh!

adangel commented Nov 24, 2020

Uh oh!

adangel commented Nov 24, 2020

Uh oh!

oowekyala commented Nov 25, 2020

Uh oh!

adangel left a comment

Choose a reason for hiding this comment

Uh oh!

adangel Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

adangel Nov 24, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

adangel commented Nov 25, 2020

Uh oh!

adangel commented Nov 25, 2020

Uh oh!

oowekyala commented Nov 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

oowekyala commented Nov 13, 2020 •

edited by adangel

Loading

ghost commented Nov 15, 2020 •

edited by ghost

Loading