Integrate the new nf-lang formatter (comment preservation, fmt directives, line wrapping)

build.gradle

@@ -34,7 +34,9 @@ configurations {
 }
 
 dependencies {
-  implementation 'io.nextflow:nf-lang:26.04.3'
+  // TEMPORARY: locally-built nf-lang from nextflow-io/nextflow#7346 (via mavenLocal).
+  // Amend this to the released nf-lang version once PR #7346 is merged and released.
+  implementation 'io.nextflow:nf-lang:26.07.0-edge'
   implementation 'org.apache.groovy:groovy:4.0.31'
   implementation 'org.apache.groovy:groovy-json:4.0.31'
   implementation 'org.apache.groovy:groovy-yaml:4.0.31'

src/main/java/nextflow/lsp/NextflowLanguageServer.java

@@ -378,7 +378,8 @@ public CompletableFuture<List<? extends TextEdit>> formatting(DocumentFormatting
                 params.getOptions().isInsertSpaces(),
                 configuration.harshilAlignment(),
                 configuration.maheshForm(),
-                configuration.sortDeclarations()
+                configuration.sortDeclarations(),
+                configuration.maxLineLength()
             );
             log.debug(String.format("textDocument/formatting %s %s %d", relativePath(uri), options.insertSpaces() ? "spaces" : "tabs", options.tabSize()));
             var service = getLanguageService(uri);
@@ -462,6 +463,7 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) {
             withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.harshilAlignment"), configuration.harshilAlignment()),
             withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.maheshForm"), configuration.maheshForm()),
             withDefault(JsonUtils.getInteger(settings, "nextflow.completion.maxItems"), configuration.maxCompletionItems()),
+            withDefault(JsonUtils.getInteger(settings, "nextflow.formatting.maxLineLength"), configuration.maxLineLength()),
             withDefault(JsonUtils.getString(settings, "nextflow.pluginRegistryUrl"), configuration.pluginRegistryUrl()),
             withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.sortDeclarations"), configuration.sortDeclarations())
         );

src/main/java/nextflow/lsp/services/FormattingProvider.java

@@ -18,11 +18,26 @@
 import java.net.URI;
 import java.util.List;
 
+import nextflow.script.formatter.CommentReattacher;
 import nextflow.script.formatter.FormattingOptions;
 import org.eclipse.lsp4j.TextEdit;
 
 public interface FormattingProvider {
 
     List<? extends TextEdit> formatting(URI uri, FormattingOptions options);
 
+    /**
+     * Determine whether formatting preserved every comment, as a safety
+     * check before returning any edit. The lexing mode must match the
+     * formatting visitor that produced the new text.
+     *
+     * @param oldText
+     * @param newText
+     * @param configFile whether to lex as a config file instead of a script
+     */
+    static boolean commentsPreserved(String oldText, String newText, boolean configFile) {
+        return CommentReattacher.commentTexts(oldText, configFile)
+            .equals(CommentReattacher.commentTexts(newText, configFile));
+    }
+
 }

src/main/java/nextflow/lsp/services/LanguageServerConfiguration.java

@@ -18,6 +18,8 @@
 import java.util.Collections;
 import java.util.List;
 
+import nextflow.script.formatter.FormattingOptions;
+
 public record LanguageServerConfiguration(
     String dagDirection,
     boolean dagVerbose,
@@ -27,6 +29,7 @@ public record LanguageServerConfiguration(
     boolean harshilAlignment,
     boolean maheshForm,
     int maxCompletionItems,
+    int maxLineLength,
     String pluginRegistryUrl,
     boolean sortDeclarations
 ) {
@@ -41,6 +44,7 @@ public static LanguageServerConfiguration defaults() {
             false,
             false,
             100,
+            FormattingOptions.DEFAULT_MAX_LINE_LENGTH,
             "https://registry.nextflow.io/api/",
             false
         );

src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java

@@ -68,10 +68,18 @@ public List<? extends TextEdit> formatting(URI uri, FormattingOptions options) {
         }
 
         var range = new Range(new Position(0, 0), Positions.getPosition(oldText, oldText.length()));
-        var visitor = new ConfigFormattingVisitor(sourceUnit, options);
+        var visitor = new ConfigFormattingVisitor(sourceUnit, options, oldText);
         visitor.visit();
         var newText = visitor.toString();
 
+        if( newText.equals(oldText) )
+            return Collections.emptyList();
+
+        if( !FormattingProvider.commentsPreserved(oldText, newText, true) ) {
+            log.showError("Refusing to format config file because formatting would remove or alter comments (please report this as a bug): " + uri);
+            return Collections.emptyList();
+        }
+
         return List.of( new TextEdit(range, newText) );
     }
 

src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java

@@ -68,10 +68,18 @@ public List<? extends TextEdit> formatting(URI uri, FormattingOptions options) {
         }
 
         var range = new Range(new Position(0, 0), Positions.getPosition(oldText, oldText.length()));
-        var visitor = new ScriptFormattingVisitor(sourceUnit, options);
+        var visitor = new ScriptFormattingVisitor(sourceUnit, options, oldText);
         visitor.visit();
         var newText = visitor.toString();
 
+        if( newText.equals(oldText) )
+            return Collections.emptyList();
+
+        if( !FormattingProvider.commentsPreserved(oldText, newText, false) ) {
+            log.showError("Refusing to format script because formatting would remove or alter comments (please report this as a bug): " + uri);
+            return Collections.emptyList();
+        }
+
         return List.of( new TextEdit(range, newText) );
     }
 

src/main/java/nextflow/lsp/services/script/ScriptService.java

@@ -158,7 +158,8 @@ private static FormattingOptions formattingOptions(LanguageServerConfiguration c
             true,
             configuration.harshilAlignment(),
             configuration.maheshForm(),
-            configuration.sortDeclarations()
+            configuration.sortDeclarations(),
+            configuration.maxLineLength()
         );
     }
 

src/test/groovy/nextflow/lsp/DiagnosticsReportingTest.groovy

@@ -43,7 +43,7 @@ class DiagnosticsReportingTest extends Specification {
         def configuration = new LanguageServerConfiguration(
             d.dagDirection(), d.dagVerbose(), mode, d.excludePatterns(),
             d.extendedCompletion(), d.harshilAlignment(), d.maheshForm(),
-            d.maxCompletionItems(), d.pluginRegistryUrl(), d.sortDeclarations())
+            d.maxCompletionItems(), d.maxLineLength(), d.pluginRegistryUrl(), d.sortDeclarations())
         def service = new ConfigService(workspaceRootUri())
         service.connect(client)
         service.initialize(configuration, new PluginSpecCache(configuration.pluginRegistryUrl()))

src/test/groovy/nextflow/lsp/TestUtils.groovy

@@ -144,4 +144,30 @@ class TestUtils {
         service.didOpen(new DidOpenTextDocumentParams(textDocumentItem))
     }
 
+    /**
+     * Open a file, also writing it to disk. Tests that issue multiple
+     * requests against the same file without a document change in between
+     * must use this instead of open(): the deferred workspace scan re-scans
+     * from disk and would evict an in-memory-only file from the AST cache.
+     *
+     * Delete the file with deleteOnDisk() in the test's cleanup block.
+     *
+     * @param service
+     * @param uri
+     * @param contents
+     */
+    static void openOnDisk(LanguageService service, String uri, String contents) {
+        Files.writeString(Path.of(URI.create(uri)), contents.stripIndent())
+        open(service, uri, contents)
+    }
+
+    /**
+     * Delete a file written by openOnDisk().
+     *
+     * @param uri
+     */
+    static void deleteOnDisk(String uri) {
+        Files.deleteIfExists(Path.of(URI.create(uri)))
+    }
+
 }

src/test/groovy/nextflow/lsp/ast/ASTNodeStringUtilsTest.groovy

@@ -74,7 +74,7 @@ class ASTNodeStringUtilsTest extends Specification {
 
         then:
         ASTNodeStringUtils.getLabel(ffn) == '(feature flag) nextflow.enable.strict'
-        ASTNodeStringUtils.getDocumentation(ffn) == 'When `true`, the pipeline is executed in [strict mode](https://nextflow.io/docs/latest/reference/feature-flags.html).'
+        ASTNodeStringUtils.getDocumentation(ffn) == 'When `true`, the pipeline is executed in [strict mode](https://docs.seqera.io/nextflow/reference/feature-flags).'
     }
 
     def 'should get the label and docs for a workflow' () {
@@ -180,7 +180,7 @@ class ASTNodeStringUtilsTest extends Specification {
         ASTNodeStringUtils.getDocumentation(node) == '''
             Create a channel that emits each argument.
 
-            [Read more](https://nextflow.io/docs/latest/reference/channel.html#of)
+            [Read more](https://docs.seqera.io/nextflow/reference/channel#of)
             '''.stripIndent(true).trim()
     }
 
@@ -202,7 +202,7 @@ class ASTNodeStringUtilsTest extends Specification {
         ASTNodeStringUtils.getDocumentation(node) == '''
             The `executor` defines the underlying system where tasks are executed.
 
-            [Read more](https://nextflow.io/docs/latest/reference/process.html#executor)
+            [Read more](https://docs.seqera.io/nextflow/reference/process#executor)
             '''.stripIndent(true).trim()
     }
 

src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy

@@ -16,63 +16,123 @@
 
 package nextflow.lsp.services.config
 
-import java.nio.file.Files
-import java.nio.file.Path
-
-import nextflow.lsp.TestLanguageClient
-import nextflow.lsp.services.LanguageServerConfiguration
 import nextflow.script.formatter.FormattingOptions
-import org.eclipse.lsp4j.DidOpenTextDocumentParams
-import org.eclipse.lsp4j.Position
-import org.eclipse.lsp4j.TextDocumentItem
 import spock.lang.Specification
 
+import static nextflow.lsp.TestUtils.*
+
 /**
  *
  * @author Ben Sherman <bentshermann@gmail.com>
  */
 class ConfigFormattingTest extends Specification {
 
-    String openAndFormat(ConfigService service, Path filePath, String contents) {
-        def uri = filePath.toUri()
-        def textDocumentItem = new TextDocumentItem(uri.toString(), 'nextflow-config', 1, contents)
-        service.didOpen(new DidOpenTextDocumentParams(textDocumentItem))
-        def textEdits = service.formatting(uri, new FormattingOptions(4, true))
-        return textEdits.first().getNewText()
+    String openAndFormat(ConfigService service, String uri, String contents) {
+        open(service, uri, contents)
+        def textEdits = service.formatting(URI.create(uri), new FormattingOptions(4, true))
+        // no edits means the document was already formatted
+        return textEdits ? textEdits.first().getNewText() : contents.stripIndent()
+    }
+
+    boolean checkFormat(ConfigService service, String uri, String before, String after) {
+        assert openAndFormat(service, uri, before) == after.stripIndent()
+        return true
+    }
+
+    boolean checkRoundTrip(ConfigService service, String uri, String source) {
+        return checkFormat(service, uri, source, source)
     }
 
     def 'should format a config file' () {
         given:
-        def workspaceRoot = Path.of(System.getProperty('user.dir')).resolve('build/test_workspace/')
-        if( !Files.exists(workspaceRoot) )
-            workspaceRoot.toFile().mkdirs()
-
-        def service = new ConfigService(workspaceRoot.toUri().toString())
-        def configuration = LanguageServerConfiguration.defaults()
-        service.connect(new TestLanguageClient())
-        service.initialize(configuration)
+        def service = getConfigService()
+        def uri = getUri('nextflow.config')
 
-        when:
-        def filePath = workspaceRoot.resolve('nextflow.config')
-        def contents = '''\
+        expect:
+        checkFormat(service, uri,
+            '''\
             process.cpus = 2 ; process.memory = 8.GB
-            '''.stripIndent()
-        then:
-        openAndFormat(service, filePath, contents) == '''\
+            ''',
+            '''\
             process.cpus = 2
             process.memory = 8.GB
-            '''.stripIndent()
-
-        when:
-        contents = '''\
+            '''
+        )
+        checkRoundTrip(service, uri,
+            '''\
             process.cpus = 2
             process.memory = 8.GB
-            '''.stripIndent()
-        then:
-        openAndFormat(service, filePath, contents) == '''\
+            '''
+        )
+    }
+
+    def 'should preserve all comments when formatting a config file' () {
+        given:
+        def service = getConfigService()
+        def uri = getUri('nextflow.config')
+
+        expect:
+        // trailing comments, dangling comments at the end of a block and at
+        // the end of the file are all preserved
+        checkRoundTrip(service, uri,
+            '''\
+            process {
+                cpus = 2 // trailing comment
+                // comment after the last option
+            }
+
+            // comment at the end of the file
+            '''
+        )
+    }
+
+    def 'should not format config regions excluded with fmt directives' () {
+        given:
+        def service = getConfigService()
+        def uri = getUri('nextflow.config')
+
+        expect:
+        checkRoundTrip(service, uri,
+            '''\
             process.cpus = 2
-            process.memory = 8.GB
+
+            // fmt: off
+            env.FOO    = 'one'
+            env.BARBAZ = 'two'
+            // fmt: on
+            '''
+        )
+    }
+
+    def 'should produce identical output when formatting a cached config AST twice' () {
+        given:
+        // formatting the same document repeatedly without a document change
+        // re-derives the comment metadata on the same cached AST -- the
+        // output must not change; the input is deliberately non-canonical
+        // so that every request returns an edit
+        def service = getConfigService()
+        def uri = getUri('nextflow.config')
+        def contents = '''\
+            // top comment
+            process {
+                cpus=2 // trailing comment
+                // dangling comment
+            }
             '''.stripIndent()
+        def expected = contents.replace('cpus=2', 'cpus = 2')
+
+        when:
+        openOnDisk(service, uri, contents)
+        def texts = (1..3).collect {
+            def edits = service.formatting(URI.create(uri), new FormattingOptions(4, true))
+            edits ? edits.first().getNewText() : contents
+        }
+
+        then:
+        texts == [expected] * 3
+
+        cleanup:
+        deleteOnDisk(uri)
     }
 
 }