You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@netbeans.apache.org by "emilianbold (via GitHub)" <gi...@apache.org> on 2026/10/21 12:12:59 UTC

[PR] Standalone tool: make document-based rewrites work [netbeans-jackpot30]

emilianbold opened a new pull request, #54:
URL: https://github.com/apache/netbeans-jackpot30/pull/54

   A rewrite that relocates original code into newly generated code — e.g. the body of a classic `for` loop that becomes an enhanced `for` — computes its diff through a scratch `Document` (`CasualDiff`). In the IDE the pieces come from layer and named-service registrations of the editor modules; the standalone jar's class closure includes **neither**, so such a rewrite is today **silently not applied**: exit 0, no output, file unchanged.
   
   Reproduce on master: build the standalone tool, then run a rule whose fix moves a multi-line body, e.g.
   
   ```
   "Enhanced for":
   for (int $i = 0; $i < $array.length; $i++) {
       $T $var = $array[$i];
       $stmts$;
   } :: !referencedIn($i, $stmts$)
   =>
   for ($T $var : $array) {
       $stmts$;
   }
   ;;
   ```
   
   with `--apply --hint-file`: the file is left unchanged and the tool reports success.
   
   What was missing, piece by piece:
   
   1. **Document services in the closure.** `StandaloneTools` now registers a `DocumentFactory` (`StandaloneDocumentFactory`) and the Java `IndentTask` (`Reindenter.Factory`) for `text/x-java`, and `BaseDocumentServices` — the `AtomicLockDocument` service for every `BaseDocument`, mirroring `org.netbeans.modules.editor.lib.DocumentServices`. `CreateStandaloneJar` packages `BaseDocumentServices` and `editor.document.StubImpl$F`, and copies the two `Editors/Documents/...DocumentServiceFactory` named-service registrations that the class-closure scan cannot discover (`lib/nbproject/project.xml` gains the `editor.lib/3` dependency and an implementation-version on `editor.document` for the accessors used).
   2. **JVM flags.** The scratch `Document` requires `-Djava.awt.headless=true` and `--add-opens=java.desktop/javax.swing.text=ALL-UNNAMED`; the launcher passes them (and accepts `$JACKPOT_OPTS`).
   3. **The rewrite was computed but never saved.** `ModificationResult.commit()` writes into the file's `Document` when one is open (the engine itself opens one when it resolves a line number); the save that follows went through the `FileObject`'s lookup, which does **not** include the `DataObject`'s `Savable` — the rewrite was lost. Save through the `DataObject` now.
   4. **Silent failures became loud.** Problems collected during apply were dropped and the tool exited 0; they are now printed to stderr and make the run exit nonzero.
   5. **Packaging fix.** `build.xml` chmod-ed the launcher *before* copying it, so the built launcher was not executable; chmod after the copy.
   
   ## Test
   
   `CreateToolTest.testHintFileDocumentBasedRewrite`: runs the standalone jar in a fresh JVM (each `CreateToolTest` test does), applies an enhanced-for rule whose fix relocates and re-indents a statement, and golden-checks the result. Silently a no-op before; correct after. The test lives in `CreateToolTest` (not `MainTest`) on purpose: it exercises the *standalone* closure, and running it in the shared in-process test JVM changes the JVM's AWT/Swing state for later tests.
   
   Test-suite state: MainTest 29/29, CreateToolTest 30/30, scripted tests pass, zip builds and the shipped launcher performs the rewrite (JDK 25, NetBeans 31 platform).
   
   ## Notes
   
   - The commit carries `Assisted-by: GLM-5.3` per the ASF generative-tooling guidance.
   - `memory:`-URL support (`FileUtil.createMemoryFileSystem()` in the standalone jar) is a separate follow-up; the named-services copying introduced here is the infrastructure it would build on.
   


-- 
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


Re: [PR] Standalone tool: make document-based rewrites work [netbeans-jackpot30]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #54:
URL: https://github.com/apache/netbeans-jackpot30/pull/54#discussion_r4336960846


##########
cmdline/tool/src/org/netbeans/modules/jackpot30/cmdline/Main.java:
##########
@@ -582,9 +584,13 @@ private static GroupResult handleGroup(RootConfiguration rootConfiguration, Prog
                 occurrences = filterBatchResult(occurrences, patch);
 
                 if (globalConfig.apply) {
-                    apply(nestedProgress, occurrences, globalConfig.out);
+                    List<MessageImpl> problems = apply(nestedProgress, occurrences, globalConfig.out);
 
-                    return GroupResult.SUCCESS; //TODO: WarningsAndErrors?
+                    for (MessageImpl problem : problems) {
+                        System.err.println(problem.kind.name().toLowerCase() + ": " + problem.text);
+                    }

Review Comment:
   `String#toLowerCase()` is locale-sensitive; on some locales (e.g. Turkish) this can produce unexpected output (and break log parsing/tests). Use `toLowerCase(Locale.ROOT)` (or avoid case conversion entirely) when formatting `problem.kind`.



##########
cmdline/lib/test/unit/src/org/netbeans/modules/jackpot30/cmdline/lib/CreateStandaloneJar.java:
##########
@@ -307,6 +311,33 @@ public InputSource resolveEntity(String publicId, String systemId) throws SAXExc
             addMETA_INFRegistration(out, info, e.getValue());
         }
 
+        //named services (@ServiceProvider(path=...)), not discoverable from the class closure:
+        Set<String> namedServices = new HashSet<String>();
+        namedServices.add("Editors/Documents/org.netbeans.editor.BaseDocument/org.netbeans.modules.editor.document.implspi.DocumentServiceFactory");
+        namedServices.add("Editors/Documents/javax.swing.text.Document/org.netbeans.modules.editor.document.implspi.DocumentServiceFactory");
+
+        for (String namedService : namedServices) {
+            String resourceName = "META-INF/namedservices/" + namedService;
+            Enumeration<URL> resources = this.getClass().getClassLoader().getResources(resourceName);
+
+            StringBuilder content = new StringBuilder();
+
+            while (resources.hasMoreElements()) {
+                //keep only implementations that are part of the standalone jar;
+                //MetaInfServicesLookup rejects the whole file if one class is missing:
+                for (String line : new String(readFile(resources.nextElement()), "UTF-8").split("\n")) {
+                    String impl = line.trim();

Review Comment:
   Prefer `StandardCharsets.UTF_8` over string charset names. It avoids checked-charset-name handling, is clearer/safer, and is consistent with modern Java practices.



##########
cmdline/lib/test/unit/src/org/netbeans/modules/jackpot30/cmdline/lib/CreateStandaloneJar.java:
##########
@@ -307,6 +311,33 @@ public InputSource resolveEntity(String publicId, String systemId) throws SAXExc
             addMETA_INFRegistration(out, info, e.getValue());
         }
 
+        //named services (@ServiceProvider(path=...)), not discoverable from the class closure:
+        Set<String> namedServices = new HashSet<String>();
+        namedServices.add("Editors/Documents/org.netbeans.editor.BaseDocument/org.netbeans.modules.editor.document.implspi.DocumentServiceFactory");
+        namedServices.add("Editors/Documents/javax.swing.text.Document/org.netbeans.modules.editor.document.implspi.DocumentServiceFactory");
+
+        for (String namedService : namedServices) {
+            String resourceName = "META-INF/namedservices/" + namedService;
+            Enumeration<URL> resources = this.getClass().getClassLoader().getResources(resourceName);
+
+            StringBuilder content = new StringBuilder();
+
+            while (resources.hasMoreElements()) {
+                //keep only implementations that are part of the standalone jar;
+                //MetaInfServicesLookup rejects the whole file if one class is missing:
+                for (String line : new String(readFile(resources.nextElement()), "UTF-8").split("\n")) {
+                    String impl = line.trim();
+                    if (impl.isEmpty() || impl.startsWith("#") || out.containsKey(escapeJavaxLang(info, impl.replace('.', '/') + ".class"))) {
+                        content.append(line).append("\n");
+                    }
+                }
+            }
+
+            if (content.length() > 0) {
+                out.put(resourceName, content.toString().getBytes("UTF-8"));

Review Comment:
   Prefer `StandardCharsets.UTF_8` over string charset names. It avoids checked-charset-name handling, is clearer/safer, and is consistent with modern Java practices.



-- 
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


Re: [PR] Standalone tool: make document-based rewrites work [netbeans-jackpot30]

Posted by "emilianbold (via GitHub)" <gi...@apache.org>.
emilianbold commented on code in PR #54:
URL: https://github.com/apache/netbeans-jackpot30/pull/54#discussion_r4347017775


##########
cmdline/tool/src/org/netbeans/modules/jackpot30/cmdline/Main.java:
##########
@@ -582,9 +584,13 @@ private static GroupResult handleGroup(RootConfiguration rootConfiguration, Prog
                 occurrences = filterBatchResult(occurrences, patch);
 
                 if (globalConfig.apply) {
-                    apply(nestedProgress, occurrences, globalConfig.out);
+                    List<MessageImpl> problems = apply(nestedProgress, occurrences, globalConfig.out);
 
-                    return GroupResult.SUCCESS; //TODO: WarningsAndErrors?
+                    for (MessageImpl problem : problems) {
+                        System.err.println(problem.kind.name().toLowerCase() + ": " + problem.text);
+                    }

Review Comment:
   This is just error logging..



-- 
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