MIGR-226: Improve the capabilities of the Java AST source analyzer #491

mbriskar · 2015-03-04T14:36:01Z

This PR adds new attribute to JavaClass, so now we will be able to query the raw line of the file like so:

JavaClass.references({*}.MySimpleClass.mySimpleMethod(java.lang.String)).matchesSource("{*}{line}").at(TypeReferenceLocation.METHOD_CALL).... where("line").matches(".methodCall(new .*")

This condition distinguish the hard-coded string inside the mySimpleMethod call.

windupgithubbot1 · 2015-03-04T15:59:23Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/565/
Test PASSed.

windupgithubbot1 · 2015-03-04T17:43:37Z

Merged build finished. Test PASSed.

windupgithubbot1 · 2015-03-04T17:43:37Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/566/
Test PASSed.

lincolnthree · 2015-03-05T19:35:49Z

rules-java/api/src/main/java/org/jboss/windup/rules/apps/java/condition/JavaClassBuilder.java

@@ -14,6 +14,11 @@
 public interface JavaClassBuilder extends ConditionBuilder
 {
    /**
+     * Specify the exact regex for the line to match (useful to distinguish hard-coded strings etc.)


It's not a regex, is it? It's a ParameterizedPattern

nor the type in inType is :-)

lincolnthree · 2015-03-05T19:57:23Z

This looks good. I'm not sure withLineMatch()is a great name for this method. How about:

JavaClass
   .references({*}.MySimpleClass.mySimpleMethod(java.lang.String))
   .matches("{*}mySimpleMethod(\"{*}\"){*)")
   .at(TypeReferenceLocation.METHOD_CALL)

mbriskar · 2015-03-05T21:54:48Z

I don't like matches that much because it doesn't say anything about what's going to be matched there and it is already used in XmlFile for xpath match

mbriskar · 2015-03-05T21:58:54Z

java-ast/addon/src/main/java/org/jboss/windup/ast/java/JavaASTProcessor.java

+
+    private String extractDefinitionLine(String typeDeclaration) {
+        String typeLine = "";
+        String[] lines = typeDeclaration.split(System.getProperty("line.separator"));


as jess/lincoln said, this is system specific and will not work in case the scanned library comes from different system

jsight · 2015-03-05T21:59:40Z

Perhaps "sourceMatches"?

sgilda · 2015-03-06T02:51:06Z

Or maybe 'matchesSource'? That way, if there are other future matches, they'll all start with matches?

mbriskar · 2015-03-06T13:26:52Z

rules-java/tests/src/test/java/org/jboss/windup/rules/java/JavaClassSourceMatchTest.java

+                    secondRuleMatchCount++;
+                }
+            }).endIteration())
+            .where("line").matches("testJavaClassCondition\\(\\) throws IOException, InstantiationException")


This regex does not match. In case you know why, I am eager to hear :) because I don't know yet.
The line registered in the graph is exactly
"
@test public void testJavaClassCondition() throws IOException, InstantiationException, IllegalAccessException {"

You have a "\n" character at the beginning and that is throwing off the regex match. Seems a little brittle doesn't it? :)

I thought {*} will help. Yes, it is brittle, however this one may be easily fixed.

mbriskar · 2015-03-06T13:28:36Z

I like the matchesSource approach

mbriskar · 2015-03-06T13:29:04Z

java-ast/addon/src/main/java/org/jboss/windup/ast/java/JavaASTProcessor.java

+            }
+        }
+        return typeLine;
+    }


Can somebody check this? I am not sure this will work system-wide

I think that it is ok, although I am not sure how I feel about including the "\n".

I think there is always only one line, so this may be taken of

Well, it could span quite a few lines in the raw text. But I think accounting for this with a regex would be painful, and the extra lines are not syntactically significant, IMO.

I think eclipse JDT forgets abouts some lines, so all the times I checked there were lines like "@OverRide public void myMethod(params) {"

That is possible. I haven't really tried it to see.

windupgithubbot1 · 2015-03-06T14:57:08Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/578/

Failed Tests: 3

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.ast.java.JavaASTProcessorTest.testHelloWorld

org.jboss.windup.rules.apps:windup-rules-java-tests: 2

Test FAILed.

jsight · 2015-03-06T19:47:35Z

As an FYI, toString() for JavaClass needs to be udpated as well.

jsight · 2015-03-06T20:35:56Z

...java/api/src/main/java/org/jboss/windup/rules/apps/java/scan/ast/JavaTypeReferenceModel.java

+     * Gets the snippit referenced by this {@link FileLocationModel}.
+     */
+    @Property(RESOLVED_SOURCE_SNIPPIT)
+    void setResolvedSourceSnippit(String source);


I think that I would prefer this field be used for the unresolved source snippit. I'm not sure why we need to change the default behavior for referencedSourceSnippit?

What do you mean exactly? I would say it is easier to understand that SOURCE_SNIPPIT is a raw part of the source and it would also correlate with the xml layer we have. BTW thanks for taking time looking at this PR

Maybe it's just "because we've always done it that way". :) It just feels strange to me to change the existing behavior for sourceSnippit (that has existed since Windup 1.x).

@lincolnthree what do you say? :) I will get used to both

@mbriskar After looking over the way we use this value more, I don't think it is a big deal to change it to your approach at all. I'm ok with it. :)

windupgithubbot1 · 2015-03-09T13:32:47Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/580/

Failed Tests: 2

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.ast.java.JavaASTProcessorTest.testHelloWorld

org.jboss.windup.rules.apps:windup-rules-java-tests: 1

org.jboss.windup.rules.java.JavaClassTest.testJavaClassCondition

Test FAILed.

windupgithubbot1 · 2015-03-09T14:48:22Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/581/

Failed Tests: 2

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.ast.java.JavaASTProcessorTest.testHelloWorld

org.jboss.windup.rules.apps:windup-rules-java-tests: 1

org.jboss.windup.rules.java.JavaClassTest.testJavaClassCondition

Test FAILed.

windupgithubbot1 · 2015-03-09T20:42:05Z

Refer to this link for build results (access rights to CI server needed):
http://10.3.8.5//job/windup-pr-builder/583/
Test PASSed.

jsight · 2015-03-09T22:00:11Z

@mbriskar Unless anyone else has objections, I think this is good to go at this point? Maybe it should wait until the metadata merge, though?

mbriskar · 2015-03-10T09:16:34Z

@jsight Both works for me

mbriskar added the Depends on another PR/Release label Mar 4, 2015

jsight added the Needs Rebase label Mar 4, 2015

mbriskar removed the Depends on another PR/Release label Mar 4, 2015

mbriskar force-pushed the MIGR-226 branch 3 times, most recently from 6ba7d37 to 2d2c5f5 Compare March 4, 2015 16:30

mbriskar added Ready to Review and removed Needs Rebase labels Mar 4, 2015

lincolnthree reviewed Mar 5, 2015
View reviewed changes

mbriskar reviewed Mar 5, 2015
View reviewed changes

mbriskar added Work in Progress/For Discussion Only and removed Ready to Review labels Mar 6, 2015

mbriskar reviewed Mar 6, 2015
View reviewed changes

jsight reviewed Mar 6, 2015
View reviewed changes

mbriskar force-pushed the MIGR-226 branch 2 times, most recently from 8e886c4 to 33e27b9 Compare March 9, 2015 12:07

mbriskar added Ready to Review and removed Work in Progress/For Discussion Only labels Mar 9, 2015

mbriskar added Work in Progress/For Discussion Only and removed Ready to Review labels Mar 9, 2015

mbriskar force-pushed the MIGR-226 branch from 33e27b9 to ebb67c6 Compare March 9, 2015 18:22

MIGR-226: Improve the capabilities of the Java AST source analyzer

1c41449

mbriskar force-pushed the MIGR-226 branch from ebb67c6 to 1c41449 Compare March 9, 2015 18:22

jsight added the Reviewed/Ready to Merge label Mar 9, 2015

mbriskar removed the Work in Progress/For Discussion Only label Mar 10, 2015

jsight merged commit 1c41449 into windup:master Mar 10, 2015

PhilipCattanach pushed a commit to PhilipCattanach/windup that referenced this pull request Feb 20, 2023

WINDUPRULE-629: spring mobile to quarkus migration rule (windup#491)

e0d4bf7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MIGR-226: Improve the capabilities of the Java AST source analyzer #491

MIGR-226: Improve the capabilities of the Java AST source analyzer #491

mbriskar commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

lincolnthree Mar 5, 2015

mbriskar Mar 6, 2015

lincolnthree commented Mar 5, 2015

mbriskar commented Mar 5, 2015

mbriskar Mar 5, 2015

jsight commented Mar 5, 2015

sgilda commented Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 6, 2015

mbriskar Mar 6, 2015

mbriskar commented Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 6, 2015

windupgithubbot1 commented Mar 6, 2015

jsight commented Mar 6, 2015

jsight Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 6, 2015

mbriskar Mar 6, 2015

jsight Mar 9, 2015

windupgithubbot1 commented Mar 9, 2015

windupgithubbot1 commented Mar 9, 2015

windupgithubbot1 commented Mar 9, 2015

jsight commented Mar 9, 2015

mbriskar commented Mar 10, 2015

MIGR-226: Improve the capabilities of the Java AST source analyzer #491

MIGR-226: Improve the capabilities of the Java AST source analyzer #491

Conversation

mbriskar commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

windupgithubbot1 commented Mar 4, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lincolnthree commented Mar 5, 2015

mbriskar commented Mar 5, 2015

Choose a reason for hiding this comment

jsight commented Mar 5, 2015

sgilda commented Mar 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbriskar commented Mar 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

windupgithubbot1 commented Mar 6, 2015

Failed Tests: 3

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.rules.apps:windup-rules-java-tests: 2

jsight commented Mar 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

windupgithubbot1 commented Mar 9, 2015

Failed Tests: 2

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.rules.apps:windup-rules-java-tests: 1

windupgithubbot1 commented Mar 9, 2015

Failed Tests: 2

org.jboss.windup.ast:windup-java-ast-tests: 1

org.jboss.windup.rules.apps:windup-rules-java-tests: 1

windupgithubbot1 commented Mar 9, 2015

jsight commented Mar 9, 2015

mbriskar commented Mar 10, 2015