You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by GitBox <gi...@apache.org> on 2022/10/01 16:45:00 UTC

[GitHub] [netbeans] errael opened a new pull request, #4711: Override gradle's compiler version from compiler args.

errael opened a new pull request, #4711:
URL: https://github.com/apache/netbeans/pull/4711

   Fix #4704. This applies to both `-source` and `-target`. This information is used by editor/hints.
   
   With this patch and the following build.gradle excerpt
   ```
   tasks.withType(JavaCompile).each {
       it.options.compilerArgs << '--source' << '19'
       it.options.compilerArgs << '--target' << '11'
       it.options.compilerArgs << '-Xlint:deprecation' << '--enable-preview'
   }
   
   sourceCompatibility = 9
   targetCompatibility = 9
   ```
   There's
   ![19-11](https://user-images.githubusercontent.com/20450427/193419462-0b7e5f94-d9d7-4c44-a21c-5a411e44345f.png)
   
   and the following has no errors in the editor.
   ```
           Object o2 = 3;
           return switch (o2) {
             case Integer i when i >= 0 -> 3; // positive integers
             case Integer i -> 4;             // negative integers
             case default -> 2;
           };
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] lkishalmi commented on a diff in pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
lkishalmi commented on code in PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#discussion_r985302978


##########
java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java:
##########
@@ -162,7 +165,36 @@ public String getTargetCompatibility() {
      * @return the defined target compatibility
      */
     public String getTargetCompatibility(SourceType type) {
-        return targetCompatibility.getOrDefault(type, getSourcesCompatibility(type));
+        return fixCompatibility(type, targetCompatibility).orElse(getSourcesCompatibility(type));
+    }
+
+    /**
+     * Use compiler arguments to override source/target compatibility for JAVA.
+     * Look for something like "-flag" or "--flag" in args, the last occurence;
+     * return  it if found.
+     * <br>
+     * For example for source, flag is "-source", and args is
+     * "... --source 13 ..." then return "13". If not in args
+     * then don't change the compatibility, return the current value.
+     */
+    private Optional<String> fixCompatibility(SourceType sourceType, Map<SourceType, String> compatibilityMap) {
+        String compatibility = compatibilityMap.get(sourceType);
+        if(sourceType == SourceType.JAVA) { // only fixup for java
+            List<String> args = getCompilerArgs(sourceType);
+            if(args != null && !args.isEmpty()) {
+                String flag = compatibilityMap == sourcesCompatibility ? "-source" : "-target";

Review Comment:
   Well, I'd reduce the number of ifs: getCompilerArgs() never returns with `null`, so line 184 could go. The double if in line 188 and 190 could be combined...



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on a diff in pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on code in PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#discussion_r985302226


##########
java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java:
##########
@@ -162,7 +165,36 @@ public String getTargetCompatibility() {
      * @return the defined target compatibility
      */
     public String getTargetCompatibility(SourceType type) {
-        return targetCompatibility.getOrDefault(type, getSourcesCompatibility(type));
+        return fixCompatibility(type, targetCompatibility).orElse(getSourcesCompatibility(type));
+    }
+
+    /**
+     * Use compiler arguments to override source/target compatibility for JAVA.
+     * Look for something like "-flag" or "--flag" in args, the last occurence;
+     * return  it if found.
+     * <br>
+     * For example for source, flag is "-source", and args is
+     * "... --source 13 ..." then return "13". If not in args
+     * then don't change the compatibility, return the current value.
+     */
+    private Optional<String> fixCompatibility(SourceType sourceType, Map<SourceType, String> compatibilityMap) {
+        String compatibility = compatibilityMap.get(sourceType);
+        if(sourceType == SourceType.JAVA) { // only fixup for java
+            List<String> args = getCompilerArgs(sourceType);
+            if(args != null && !args.isEmpty()) {
+                String flag = compatibilityMap == sourcesCompatibility ? "-source" : "-target";

Review Comment:
   The first implementation did that (the one that was in the wrong place). I'll make that change after any other suggested changes have time to roll in. Thanks for the comment.



##########
java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java:
##########
@@ -162,7 +165,36 @@ public String getTargetCompatibility() {
      * @return the defined target compatibility
      */
     public String getTargetCompatibility(SourceType type) {
-        return targetCompatibility.getOrDefault(type, getSourcesCompatibility(type));
+        return fixCompatibility(type, targetCompatibility).orElse(getSourcesCompatibility(type));
+    }
+
+    /**
+     * Use compiler arguments to override source/target compatibility for JAVA.
+     * Look for something like "-flag" or "--flag" in args, the last occurence;
+     * return  it if found.
+     * <br>
+     * For example for source, flag is "-source", and args is
+     * "... --source 13 ..." then return "13". If not in args
+     * then don't change the compatibility, return the current value.
+     */
+    private Optional<String> fixCompatibility(SourceType sourceType, Map<SourceType, String> compatibilityMap) {
+        String compatibility = compatibilityMap.get(sourceType);
+        if(sourceType == SourceType.JAVA) { // only fixup for java
+            List<String> args = getCompilerArgs(sourceType);
+            if(args != null && !args.isEmpty()) {
+                String flag = compatibilityMap == sourcesCompatibility ? "-source" : "-target";

Review Comment:
   I remember now why I compared to map instance; because -source/-target are/might-be specific to java. I'll change the method name to `fixJavaCompatibility`. I'd like to take out the souceType argument, but then the check for JAVA would, would have to be in the caller.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264731985

   I thought a builder would be good; immutable description. Looking at your two proposed locations, couldn't get breakpoints to work in NbProjectInfoBuilder, so that made up my mind.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264496983

   please label PRs before creating them. This will be important in future #4431, otherwise this PR would run no gradle jobs for example.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264508129

   > please label PRs before creating them
   
   I knew about the labeling and CI. I'll try to keep that in mind. I didn't think I was able to change/add labels.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on a diff in pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on code in PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#discussion_r985311719


##########
java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java:
##########
@@ -162,7 +165,36 @@ public String getTargetCompatibility() {
      * @return the defined target compatibility
      */
     public String getTargetCompatibility(SourceType type) {
-        return targetCompatibility.getOrDefault(type, getSourcesCompatibility(type));
+        return fixCompatibility(type, targetCompatibility).orElse(getSourcesCompatibility(type));
+    }
+
+    /**
+     * Use compiler arguments to override source/target compatibility for JAVA.
+     * Look for something like "-flag" or "--flag" in args, the last occurence;
+     * return  it if found.
+     * <br>
+     * For example for source, flag is "-source", and args is
+     * "... --source 13 ..." then return "13". If not in args
+     * then don't change the compatibility, return the current value.
+     */
+    private Optional<String> fixCompatibility(SourceType sourceType, Map<SourceType, String> compatibilityMap) {
+        String compatibility = compatibilityMap.get(sourceType);
+        if(sourceType == SourceType.JAVA) { // only fixup for java
+            List<String> args = getCompilerArgs(sourceType);
+            if(args != null && !args.isEmpty()) {
+                String flag = compatibilityMap == sourcesCompatibility ? "-source" : "-target";

Review Comment:
   K



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264509103

   > > please label PRs before creating them
   > 
   > I knew about the labeling and CI. I'll try to keep that in mind. I didn't think I was able to change/add labels.
   
   oh my mistake - disregard what i said. I was sure you were already committer since you are so active (which is great!). I think you have to have at least committer role to be able to label things.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264940826

   Yikes. Thanks for something real/detectable.
   
   What to do if `--release` and one of the others? In any event, a compile will fail.
   
   Arbitrarily use the value with `--release`. Unless want to consider error dialogs or somesuch, but then why stop there...


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264955458

   > What to do if `--release` and one of the others? In any event, a compile will fail.
   
   it replaces both, if you mix, javac should complain and fail as far as I know. It is similar to setting both source and target to the same value, but more powerful since javac will use ct.sym files which are basically method/class signature diffs and check that your code could actually run on the target JDK APIs.
   
   The maven template uses the source/target tags too still, if we end up adding some kind of JDK or language level selector to the new maven project wizard we could let it use that too if java 9+ is selected.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] errael commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
errael commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264732821

   > [errael](https://github.com/errael) requested review from [lkishalmi](https://github.com/lkishalmi) and removed request for [sdedic](https://github.com/sdedic) 7 minutes ago
   
   Not sure how I removed the request for @sdedic, not my intention. github bug?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] mbien commented on pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
mbien commented on PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#issuecomment-1264800068

   i am no heavy gradle user. But there is also the `--release` flag (JEP 247) which is in many ways better than using source+target since it enforces bytecode *and* API compatibility and avoids a bunch of nasty bugs. Not sure if this is relevant here, e.g what if the build sets release not source - is this handled too?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists


[GitHub] [netbeans] sdedic commented on a diff in pull request #4711: Override gradle's compiler version from compiler args.

Posted by GitBox <gi...@apache.org>.
sdedic commented on code in PR #4711:
URL: https://github.com/apache/netbeans/pull/4711#discussion_r985298896


##########
java/gradle.java/src/org/netbeans/modules/gradle/java/api/GradleJavaSourceSet.java:
##########
@@ -162,7 +165,36 @@ public String getTargetCompatibility() {
      * @return the defined target compatibility
      */
     public String getTargetCompatibility(SourceType type) {
-        return targetCompatibility.getOrDefault(type, getSourcesCompatibility(type));
+        return fixCompatibility(type, targetCompatibility).orElse(getSourcesCompatibility(type));
+    }
+
+    /**
+     * Use compiler arguments to override source/target compatibility for JAVA.
+     * Look for something like "-flag" or "--flag" in args, the last occurence;
+     * return  it if found.
+     * <br>
+     * For example for source, flag is "-source", and args is
+     * "... --source 13 ..." then return "13". If not in args
+     * then don't change the compatibility, return the current value.
+     */
+    private Optional<String> fixCompatibility(SourceType sourceType, Map<SourceType, String> compatibilityMap) {
+        String compatibility = compatibilityMap.get(sourceType);
+        if(sourceType == SourceType.JAVA) { // only fixup for java
+            List<String> args = getCompilerArgs(sourceType);
+            if(args != null && !args.isEmpty()) {
+                String flag = compatibilityMap == sourcesCompatibility ? "-source" : "-target";

Review Comment:
   Nitpick: maybe pass the flag source/target explicitly rather than comparing Map instances ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@netbeans.apache.org
For additional commands, e-mail: notifications-help@netbeans.apache.org

For further information about the NetBeans mailing lists, visit:
https://cwiki.apache.org/confluence/display/NETBEANS/Mailing+lists