GROOVY-11680: Java stubs use deprecated constant constructors in some… #2235
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR addresses GROOVY-11680 by removing usage of deprecated constant constructors in Java stub generation. Key changes include:
- Updating tests in Groovy11019.groovy to expect string constants as null.
- Adjusting test expectations and adding a new dynamic string constant in Groovy10902.groovy.
- Revising JavaStubGenerator.java to generate stub values via valueOf() for primitives while defaulting strings to null.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/test/groovy/org/codehaus/groovy/tools/stubgenerator/Groovy11019.groovy | Updated expected stub for a static string constant |
| src/test/groovy/org/codehaus/groovy/tools/stubgenerator/Groovy10902.groovy | Updated expected stub for an Integer constant, added dynamic string constant test |
| src/test/groovy/org/codehaus/groovy/tools/stubgenerator/ClassWithPrimitiveFieldsStubTest.groovy | Added tests for default initializations of various primitive fields |
| src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java | Modified generation of constant values for primitives and strings |
src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
Outdated
Show resolved
Hide resolved
|
@eric-milles I removed the special handling for String constants and it didn't seem to have any negative consequences. I'd be interested in your thoughts since you have done a few recent changes in this area. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2235 +/- ##
==================================================
- Coverage 69.0430% 69.0406% -0.0024%
- Complexity 29704 29705 +1
==================================================
Files 1423 1423
Lines 114404 114421 +17
Branches 19852 19855 +3
==================================================
+ Hits 78988 78997 +9
- Misses 28781 28786 +5
- Partials 6635 6638 +3
🚀 New features to boost your workflow:
|
src/test/groovy/org/codehaus/groovy/tools/stubgenerator/ClassWithPrimitiveFieldsStubTest.groovy
Show resolved
Hide resolved
|
The constructors are indeed deprecated, but are they scheduled for removal? Was this a concern for someone? As long as you generate something that javac accepts but does not inline, then I guess it's fine. I'd prefer it stays simple. No need to be overly complicated just for the sake of stubs. |
|
The Grails build messages are cluttered with many of these:
So, at least the Boolean constructor is marked for removal. |
|
No good deed is too small to do; no evil deed is too small to avoid. |
|
If there is a |
|
Can you include the grails build log notes in the JIRA ticket? |
| printValue(out, type, defaultValueX(type)); | ||
| out.print(')'); | ||
| } else if (isStringType(type)) { | ||
| out.print("new String()"); |
There was a problem hiding this comment.
I'd keep the qualifier. Although, a valueOf expression would not be that difficult to produce.
|
change looks fine now |
… cases