From d38c676e44bab8af2148139f3be336a2ef885afb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 12:59:30 +0000 Subject: [PATCH 1/7] TEMPORARY: consume locally-built nf-lang 26.07.0-edge (nextflow-io/nextflow#7346) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Points the build at the nf-lang formatter overhaul from PR nextflow-io/nextflow#7346, published to Maven local from the PR head branch. This commit must be amended to the released nf-lang version once the PR is merged and an nf-lang release exists — do not ship it as-is. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- build.gradle | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 95dfb43..9fd1046 100644 --- a/build.gradle +++ b/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' From c92ef0ca6fbb77a794d516ec985dab221ac6edb0 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:06:32 +0000 Subject: [PATCH 2/7] Add maxLineLength formatting option Expose the new nf-lang line-wrapping option as nextflow.formatting.maxLineLength (default FormattingOptions.DEFAULT_MAX_LINE_LENGTH = 120, 0 disables wrapping) and pass it explicitly into every FormattingOptions construction, so the language server never relies on the 5-arg constructor's implicit default. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- src/main/java/nextflow/lsp/NextflowLanguageServer.java | 4 +++- .../nextflow/lsp/services/LanguageServerConfiguration.java | 4 ++++ src/main/java/nextflow/lsp/services/script/ScriptService.java | 3 ++- src/test/groovy/nextflow/lsp/DiagnosticsReportingTest.groovy | 2 +- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/main/java/nextflow/lsp/NextflowLanguageServer.java b/src/main/java/nextflow/lsp/NextflowLanguageServer.java index 58ae0ff..53694bd 100644 --- a/src/main/java/nextflow/lsp/NextflowLanguageServer.java +++ b/src/main/java/nextflow/lsp/NextflowLanguageServer.java @@ -378,7 +378,8 @@ public CompletableFuture> 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()) ); diff --git a/src/main/java/nextflow/lsp/services/LanguageServerConfiguration.java b/src/main/java/nextflow/lsp/services/LanguageServerConfiguration.java index 14ab0d1..2981709 100644 --- a/src/main/java/nextflow/lsp/services/LanguageServerConfiguration.java +++ b/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 ); diff --git a/src/main/java/nextflow/lsp/services/script/ScriptService.java b/src/main/java/nextflow/lsp/services/script/ScriptService.java index 6c8356d..b31603c 100644 --- a/src/main/java/nextflow/lsp/services/script/ScriptService.java +++ b/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() ); } diff --git a/src/test/groovy/nextflow/lsp/DiagnosticsReportingTest.groovy b/src/test/groovy/nextflow/lsp/DiagnosticsReportingTest.groovy index 22e9089..900f626 100644 --- a/src/test/groovy/nextflow/lsp/DiagnosticsReportingTest.groovy +++ b/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())) From 2e93c3d0e4bb0e9c9c7d29f0cdb5f340c93e1955 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:06:33 +0000 Subject: [PATCH 3/7] Pass open document text to formatting visitors, refuse to format on comment loss Use the new 3-arg visitor constructors so comment reattachment always sees exactly the buffer being formatted instead of re-reading the SourceUnit. Adopt the same safety check as 'nextflow lint -format': compare CommentReattacher.commentTexts before and after formatting and refuse to return any edit if a comment would be removed or altered. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- .../lsp/services/config/ConfigFormattingProvider.java | 9 ++++++++- .../lsp/services/script/ScriptFormattingProvider.java | 9 ++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java b/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java index 8bd9030..68b75d2 100644 --- a/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java +++ b/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java @@ -24,6 +24,7 @@ import nextflow.lsp.services.FormattingProvider; import nextflow.lsp.util.Logger; import nextflow.lsp.util.Positions; +import nextflow.script.formatter.CommentReattacher; import nextflow.script.formatter.FormattingOptions; import org.codehaus.groovy.runtime.IOGroovyMethods; import org.eclipse.lsp4j.Position; @@ -68,10 +69,16 @@ public List 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) + && !CommentReattacher.commentTexts(oldText, true).equals(CommentReattacher.commentTexts(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) ); } diff --git a/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java b/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java index f3f4ccd..7a2cd83 100644 --- a/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java +++ b/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java @@ -23,6 +23,7 @@ import nextflow.lsp.services.FormattingProvider; import nextflow.lsp.util.Logger; import nextflow.lsp.util.Positions; +import nextflow.script.formatter.CommentReattacher; import nextflow.script.formatter.FormattingOptions; import nextflow.script.formatter.ScriptFormattingVisitor; import org.codehaus.groovy.runtime.IOGroovyMethods; @@ -68,10 +69,16 @@ public List 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) + && !CommentReattacher.commentTexts(oldText, false).equals(CommentReattacher.commentTexts(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) ); } From c99ee156861f73f26b1ab8f518423342d6076f14 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:17:22 +0000 Subject: [PATCH 4/7] Update formatting tests for the new nf-lang formatter Pin the new canonical style at the language-server level: K&R if/else, blank-line normalization, comment preservation (trailing, dangling and commented-out declarations), line wrapping at the configured maximum line length, and fmt: skip / fmt: off / fmt: on directives. Add integration tests for the language-server trigger scenarios: - formatting the same cached AST repeatedly without a document change produces identical output (comment metadata re-derivation must be idempotent) - maxLineLength=0 disables line wrapping - a file using fmt: off/on round-trips unchanged - converting a script to static types after formatting the same cached AST neither crashes nor duplicates comments (the conversion formats freshly-built AST nodes that carry no reattached comment metadata) The cached-AST tests write the file to disk first: the deferred workspace scan re-scans from disk and would otherwise evict an in-memory-only file from the AST cache between requests. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- .../config/ConfigFormattingTest.groovy | 113 ++++++- .../ConvertScriptStaticTypesTest.groovy | 50 +++ .../script/ScriptFormattingTest.groovy | 289 +++++++++++++++++- 3 files changed, 439 insertions(+), 13 deletions(-) diff --git a/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy b/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy index cd61c2c..7577cdc 100644 --- a/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy @@ -33,16 +33,7 @@ import spock.lang.Specification */ 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() - } - - def 'should format a config file' () { - given: + ConfigService newConfigService() { def workspaceRoot = Path.of(System.getProperty('user.dir')).resolve('build/test_workspace/') if( !Files.exists(workspaceRoot) ) workspaceRoot.toFile().mkdirs() @@ -51,9 +42,31 @@ class ConfigFormattingTest extends Specification { def configuration = LanguageServerConfiguration.defaults() service.connect(new TestLanguageClient()) service.initialize(configuration) + return service + } + + Path configPath() { + return Path.of(System.getProperty('user.dir')).resolve('build/test_workspace/nextflow.config') + } + + void openFile(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)) + } + + String openAndFormat(ConfigService service, Path filePath, String contents) { + openFile(service, filePath, contents) + def textEdits = service.formatting(filePath.toUri(), new FormattingOptions(4, true)) + return textEdits.first().getNewText() + } + + def 'should format a config file' () { + given: + def service = newConfigService() when: - def filePath = workspaceRoot.resolve('nextflow.config') + def filePath = configPath() def contents = '''\ process.cpus = 2 ; process.memory = 8.GB '''.stripIndent() @@ -75,4 +88,82 @@ class ConfigFormattingTest extends Specification { '''.stripIndent() } + def 'should preserve all comments when formatting a config file' () { + given: + def service = newConfigService() + def filePath = configPath() + + expect: + // trailing comments, dangling comments at the end of a block and at + // the end of the file are all preserved + openAndFormat(service, filePath, '''\ + process { + cpus = 2 // trailing comment + // comment after the last option + } + + // comment at the end of the file + '''.stripIndent() + ) == '''\ + process { + cpus = 2 // trailing comment + // comment after the last option + } + + // comment at the end of the file + '''.stripIndent() + } + + def 'should not format config regions excluded with fmt directives' () { + given: + def service = newConfigService() + def filePath = configPath() + + expect: + openAndFormat(service, filePath, '''\ + process.cpus = 2 + + // fmt: off + env.FOO = 'one' + env.BARBAZ = 'two' + // fmt: on + '''.stripIndent() + ) == '''\ + process.cpus = 2 + + // fmt: off + env.FOO = 'one' + env.BARBAZ = 'two' + // fmt: on + '''.stripIndent() + } + + def 'should produce identical output when formatting a cached config AST twice' () { + given: + def service = newConfigService() + def filePath = configPath() + def contents = '''\ + // top comment + process { + cpus = 2 // trailing comment + // dangling comment + } + '''.stripIndent() + // the file must exist on disk so that the deferred workspace scan + // does not evict it from the AST cache between formatting calls + Files.writeString(filePath, contents) + + when: + openFile(service, filePath, contents) + def edits = (1..3).collect { + service.formatting(filePath.toUri(), new FormattingOptions(4, true)) + } + + then: + edits.every { it.first().getNewText() == contents } + + cleanup: + Files.deleteIfExists(filePath) + } + } diff --git a/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy index af5db8f..7785e7a 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy @@ -17,6 +17,7 @@ package nextflow.lsp.services.script import nextflow.lsp.services.LanguageServerConfiguration +import nextflow.script.formatter.FormattingOptions import spock.lang.Specification import static nextflow.lsp.TestUtils.* @@ -222,6 +223,55 @@ class ConvertScriptStaticTypesTest extends Specification { checkOutputs(service, 'stdout', 'stdout()') } + def 'should convert a script after formatting the same cached AST' () { + given: + // formatting re-derives comment metadata on the cached AST; a + // subsequent conversion of the same AST must neither crash nor + // duplicate comments into the replacement text + def service = getScriptService() + def uri = getUri('main.nf') + def contents = '''\ + // about this process + process test { + input: + val id + + script: + "true" + } + + workflow { + test(1) + } + '''.stripIndent() + + // the file must exist on disk so that the deferred workspace scan + // does not evict it from the AST cache between requests + java.nio.file.Files.writeString(java.nio.file.Path.of(URI.create(uri)), contents) + + when: 'format the document so comment metadata is re-derived' + open(service, uri, contents) + def edits = service.formatting(URI.create(uri), new FormattingOptions(4, true)) + then: + edits.first().getNewText() == contents + + when: 'convert the same cached AST to static types' + def response = service.executeCommand('nextflow.server.convertScriptToTyped', [asJson(uri)], LanguageServerConfiguration.defaults()) + def newText = response.applyEdit.getChanges()[uri][0].getNewText() + then: 'the replacement text does not duplicate the leading comment, which stays in the document above the replaced range' + newText == '''\ + process test { + input: + id + + script: + "true" + }'''.stripIndent() + + cleanup: + java.nio.file.Files.deleteIfExists(java.nio.file.Path.of(URI.create(uri))) + } + def 'should convert tuple outputs' () { given: def service = getScriptService() diff --git a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy index 6a18d0e..82eaecc 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy @@ -16,6 +16,9 @@ package nextflow.lsp.services.script +import java.nio.file.Files +import java.nio.file.Path + import nextflow.script.formatter.FormattingOptions import spock.lang.Specification @@ -27,13 +30,17 @@ import static nextflow.lsp.TestUtils.* */ class ScriptFormattingTest extends Specification { - boolean checkFormat(ScriptService service, String uri, String before, String after) { + boolean checkFormat(ScriptService service, String uri, FormattingOptions options, String before, String after) { open(service, uri, before) - def textEdits = service.formatting(URI.create(uri), new FormattingOptions(4, true)) + def textEdits = service.formatting(URI.create(uri), options) assert textEdits.first().getNewText() == after.stripIndent() return true } + boolean checkFormat(ScriptService service, String uri, String before, String after) { + return checkFormat(service, uri, new FormattingOptions(4, true), before, after) + } + def 'should format a script' () { given: def service = getScriptService() @@ -93,4 +100,282 @@ class ScriptFormattingTest extends Specification { ) } + def 'should format if-else statements in K&R style' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + + expect: + checkFormat(service, uri, + '''\ + workflow { + if( params.a ) { + run_a() + } + else if( params.b ) { + run_b() + } + else { + run_c() + } + } + ''', + '''\ + workflow { + if (params.a) { + run_a() + } else if (params.b) { + run_b() + } else { + run_c() + } + } + ''' + ) + } + + def 'should normalize blank lines' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + + expect: + checkFormat(service, uri, + '''\ + include { A } from './a.nf' + include { B } from './b.nf' + params.x = 1 + workflow { + A() + } + ''', + '''\ + include { A } from './a.nf' + include { B } from './b.nf' + + params.x = 1 + + workflow { + A() + } + ''' + ) + checkFormat(service, uri, + '''\ + workflow { + + x = 1 + + + + y = 2 + } + ''', + '''\ + workflow { + x = 1 + + y = 2 + } + ''' + ) + } + + def 'should preserve all comments when formatting' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + + expect: + // trailing comments, dangling comments at the end of a block and at + // the end of the file are all preserved + checkFormat(service, uri, + '''\ + workflow { + x = 1 // trailing comment + // comment after the last statement + } + + // comment at the end of the file + ''', + '''\ + workflow { + x = 1 // trailing comment + // comment after the last statement + } + + // comment at the end of the file + ''' + ) + // a commented-out process is preserved + checkFormat(service, uri, + '''\ + // process FOO { + // script: + // "true" + // } + + process BAR { + script: + "true" + } + + workflow { + BAR() + } + ''', + '''\ + // process FOO { + // script: + // "true" + // } + + process BAR { + script: + "true" + } + + workflow { + BAR() + } + ''' + ) + } + + def 'should wrap lines that exceed the maximum line length' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + def options = new FormattingOptions(4, true, false, false, false, 60) + + expect: + checkFormat(service, uri, options, + '''\ + workflow { + ALIGN_AND_SORT(samples_channel, reference_genome, annotation_file, params.threads) + } + ''', + '''\ + workflow { + ALIGN_AND_SORT( + samples_channel, + reference_genome, + annotation_file, + params.threads, + ) + } + ''' + ) + } + + def 'should not wrap lines when the maximum line length is zero' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + def options = new FormattingOptions(4, true, false, false, false, 0) + + expect: + checkFormat(service, uri, options, + '''\ + workflow { + ALIGN_AND_SORT(samples_channel, reference_genome, annotation_file, params.threads, extra_arg_one, extra_arg_two, extra_arg_three) + } + ''', + '''\ + workflow { + ALIGN_AND_SORT(samples_channel, reference_genome, annotation_file, params.threads, extra_arg_one, extra_arg_two, extra_arg_three) + } + ''' + ) + } + + def 'should not format regions excluded with fmt directives' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + + expect: + checkFormat(service, uri, + '''\ + workflow { + x = [1, 2, 3] // fmt: skip + y = [4,5] + } + ''', + '''\ + workflow { + x = [1, 2, 3] // fmt: skip + y = [4, 5] + } + ''' + ) + // a fmt: off / fmt: on region round-trips unchanged + checkFormat(service, uri, + '''\ + workflow { + a = 1 + + // fmt: off + matrix = [ + [1, 0], + [0, 1] ] + // fmt: on + + b = 2 + } + ''', + '''\ + workflow { + a = 1 + + // fmt: off + matrix = [ + [1, 0], + [0, 1] ] + // fmt: on + + b = 2 + } + ''' + ) + } + + def 'should produce identical output when formatting a cached 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, including for comments inside wrapped + // expressions + def service = getScriptService() + def uri = getUri('main.nf') + def contents = '''\ + workflow { + foo( + // leading comment on element + alpha, + beta, // trailing comment on element + ) + data + // comment on chain link + .map { x -> x } + .view() + } + '''.stripIndent() + // the file must exist on disk so that the deferred workspace scan + // does not evict it from the AST cache between formatting calls + Files.writeString(Path.of(URI.create(uri)), contents) + + when: + open(service, uri, contents) + def edits = (1..3).collect { + service.formatting(URI.create(uri), new FormattingOptions(4, true)) + } + + then: + edits.every { it.first().getNewText() == contents } + + cleanup: + Files.deleteIfExists(Path.of(URI.create(uri))) + } + } From 03848ecf1f2022cae907a46ba2f4e2d0264c3b5d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:19:36 +0000 Subject: [PATCH 5/7] Add formatting tests for include sorting, string re-indent, CRLF comments Pin the language-server-side behavior for the remaining formatter issues closed by nextflow-io/nextflow#7346: include sorting under sortDeclarations (#54), multi-line string re-indentation (#116), and leading comments in CRLF files no longer merging with the following code (#127). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- .../script/ScriptFormattingTest.groovy | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy index 82eaecc..0b6f4d6 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy @@ -340,6 +340,77 @@ class ScriptFormattingTest extends Specification { ) } + def 'should sort includes when sorting is enabled' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + def options = new FormattingOptions(4, true, false, false, true, 120) + + expect: + checkFormat(service, uri, options, + '''\ + include { ZULU } from './modules/zulu.nf' + include { ALPHA } from './modules/alpha.nf' + + workflow { + ALPHA() + } + ''', + '''\ + include { ALPHA } from './modules/alpha.nf' + include { ZULU } from './modules/zulu.nf' + + workflow { + ALPHA() + } + ''' + ) + } + + def 'should re-indent multi-line strings' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + def options = new FormattingOptions(2, true, false, false, false, 120) + + expect: + checkFormat(service, uri, options, + '''\ + process foo { + script: + """ + echo 'hello world!' + """ + } + ''', + '''\ + process foo { + script: + """ + echo 'hello world!' + """ + } + ''' + ) + } + + def 'should format leading comments in a file with CRLF line endings' () { + given: + def service = getScriptService() + def uri = getUri('main.nf') + + expect: + checkFormat(service, uri, + "workflow {\r\n // ALIGN reads to reference genome\r\n BWA_ALIGN(sample_id, library_id)\r\n}\r\n", + '''\ + workflow { + // ALIGN reads to reference genome + BWA_ALIGN(sample_id, library_id) + } + ''' + ) + } + def 'should produce identical output when formatting a cached AST twice' () { given: // formatting the same document repeatedly without a document change From 11dd7e29ce2c04795f94519f60f652322acbcb9e Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:22:25 +0000 Subject: [PATCH 6/7] Update expected doc links for new nf-lang The Nextflow docs moved from nextflow.io/docs/latest to docs.seqera.io/nextflow upstream, so the doc links baked into nf-lang 26.07.0-edge annotations changed accordingly. This change is durable: it applies to any nf-lang release cut from current master, independent of the formatter overhaul. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- .../groovy/nextflow/lsp/ast/ASTNodeStringUtilsTest.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/groovy/nextflow/lsp/ast/ASTNodeStringUtilsTest.groovy b/src/test/groovy/nextflow/lsp/ast/ASTNodeStringUtilsTest.groovy index 861afba..0798489 100644 --- a/src/test/groovy/nextflow/lsp/ast/ASTNodeStringUtilsTest.groovy +++ b/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() } From 7f371ee4d0f7fd2e3f54e11a409b34672c58f48d Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 16 Jul 2026 13:42:01 +0000 Subject: [PATCH 7/7] Simplify formatting guard and test scaffolding - Extract the comment-preservation check into a shared FormattingProvider.commentsPreserved helper so the script and config providers no longer duplicate the safety-critical guard, and return no edits when the formatted text is unchanged instead of a whole-document no-op edit. - Move the file-on-disk workaround for the deferred workspace scan into TestUtils (openOnDisk/deleteOnDisk) so the eviction rationale is documented once instead of in three test classes. - Rebuild ConfigFormattingTest on the shared TestUtils harness instead of a private service-bootstrap clone. - Collapse round-trip test cases onto checkRoundTrip helpers instead of pasting identical literals twice, and make the cached-AST tests use non-canonical input so a refused format cannot be mistaken for a no-op. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01L6xXG4ocbea6AW3f9hYsJb --- .../lsp/services/FormattingProvider.java | 15 ++ .../config/ConfigFormattingProvider.java | 7 +- .../script/ScriptFormattingProvider.java | 7 +- src/test/groovy/nextflow/lsp/TestUtils.groovy | 26 ++++ .../config/ConfigFormattingTest.groovy | 133 +++++++----------- .../ConvertScriptStaticTypesTest.groovy | 12 +- .../script/ScriptFormattingTest.groovy | 83 ++++------- 7 files changed, 129 insertions(+), 154 deletions(-) diff --git a/src/main/java/nextflow/lsp/services/FormattingProvider.java b/src/main/java/nextflow/lsp/services/FormattingProvider.java index 0e27fb8..737f009 100644 --- a/src/main/java/nextflow/lsp/services/FormattingProvider.java +++ b/src/main/java/nextflow/lsp/services/FormattingProvider.java @@ -18,6 +18,7 @@ import java.net.URI; import java.util.List; +import nextflow.script.formatter.CommentReattacher; import nextflow.script.formatter.FormattingOptions; import org.eclipse.lsp4j.TextEdit; @@ -25,4 +26,18 @@ public interface FormattingProvider { List 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)); + } + } diff --git a/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java b/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java index 68b75d2..42eb82d 100644 --- a/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java +++ b/src/main/java/nextflow/lsp/services/config/ConfigFormattingProvider.java @@ -24,7 +24,6 @@ import nextflow.lsp.services.FormattingProvider; import nextflow.lsp.util.Logger; import nextflow.lsp.util.Positions; -import nextflow.script.formatter.CommentReattacher; import nextflow.script.formatter.FormattingOptions; import org.codehaus.groovy.runtime.IOGroovyMethods; import org.eclipse.lsp4j.Position; @@ -73,8 +72,10 @@ public List formatting(URI uri, FormattingOptions options) { visitor.visit(); var newText = visitor.toString(); - if( !newText.equals(oldText) - && !CommentReattacher.commentTexts(oldText, true).equals(CommentReattacher.commentTexts(newText, true)) ) { + 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(); } diff --git a/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java b/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java index 7a2cd83..67b2154 100644 --- a/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java +++ b/src/main/java/nextflow/lsp/services/script/ScriptFormattingProvider.java @@ -23,7 +23,6 @@ import nextflow.lsp.services.FormattingProvider; import nextflow.lsp.util.Logger; import nextflow.lsp.util.Positions; -import nextflow.script.formatter.CommentReattacher; import nextflow.script.formatter.FormattingOptions; import nextflow.script.formatter.ScriptFormattingVisitor; import org.codehaus.groovy.runtime.IOGroovyMethods; @@ -73,8 +72,10 @@ public List formatting(URI uri, FormattingOptions options) { visitor.visit(); var newText = visitor.toString(); - if( !newText.equals(oldText) - && !CommentReattacher.commentTexts(oldText, false).equals(CommentReattacher.commentTexts(newText, false)) ) { + 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(); } diff --git a/src/test/groovy/nextflow/lsp/TestUtils.groovy b/src/test/groovy/nextflow/lsp/TestUtils.groovy index 152b636..82edf6e 100644 --- a/src/test/groovy/nextflow/lsp/TestUtils.groovy +++ b/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))) + } + } diff --git a/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy b/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy index 7577cdc..30539af 100644 --- a/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/config/ConfigFormattingTest.groovy @@ -16,154 +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 */ class ConfigFormattingTest extends Specification { - ConfigService newConfigService() { - 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) - return service - } - - Path configPath() { - return Path.of(System.getProperty('user.dir')).resolve('build/test_workspace/nextflow.config') + 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() } - void openFile(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)) + boolean checkFormat(ConfigService service, String uri, String before, String after) { + assert openAndFormat(service, uri, before) == after.stripIndent() + return true } - String openAndFormat(ConfigService service, Path filePath, String contents) { - openFile(service, filePath, contents) - def textEdits = service.formatting(filePath.toUri(), new FormattingOptions(4, true)) - return textEdits.first().getNewText() + boolean checkRoundTrip(ConfigService service, String uri, String source) { + return checkFormat(service, uri, source, source) } def 'should format a config file' () { given: - def service = newConfigService() + def service = getConfigService() + def uri = getUri('nextflow.config') - when: - def filePath = configPath() - 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) == '''\ - process.cpus = 2 - process.memory = 8.GB - '''.stripIndent() + ''' + ) } def 'should preserve all comments when formatting a config file' () { given: - def service = newConfigService() - def filePath = configPath() + 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 - openAndFormat(service, filePath, '''\ + checkRoundTrip(service, uri, + '''\ process { cpus = 2 // trailing comment // comment after the last option } // comment at the end of the file - '''.stripIndent() - ) == '''\ - process { - cpus = 2 // trailing comment - // comment after the last option - } - - // comment at the end of the file - '''.stripIndent() + ''' + ) } def 'should not format config regions excluded with fmt directives' () { given: - def service = newConfigService() - def filePath = configPath() + def service = getConfigService() + def uri = getUri('nextflow.config') expect: - openAndFormat(service, filePath, '''\ + checkRoundTrip(service, uri, + '''\ process.cpus = 2 // fmt: off env.FOO = 'one' env.BARBAZ = 'two' // fmt: on - '''.stripIndent() - ) == '''\ - process.cpus = 2 - - // fmt: off - env.FOO = 'one' - env.BARBAZ = 'two' - // fmt: on - '''.stripIndent() + ''' + ) } def 'should produce identical output when formatting a cached config AST twice' () { given: - def service = newConfigService() - def filePath = configPath() + // 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 + cpus=2 // trailing comment // dangling comment } '''.stripIndent() - // the file must exist on disk so that the deferred workspace scan - // does not evict it from the AST cache between formatting calls - Files.writeString(filePath, contents) + def expected = contents.replace('cpus=2', 'cpus = 2') when: - openFile(service, filePath, contents) - def edits = (1..3).collect { - service.formatting(filePath.toUri(), new FormattingOptions(4, true)) + 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: - edits.every { it.first().getNewText() == contents } + texts == [expected] * 3 cleanup: - Files.deleteIfExists(filePath) + deleteOnDisk(uri) } } diff --git a/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy index 7785e7a..3a1d6f0 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ConvertScriptStaticTypesTest.groovy @@ -245,15 +245,11 @@ class ConvertScriptStaticTypesTest extends Specification { } '''.stripIndent() - // the file must exist on disk so that the deferred workspace scan - // does not evict it from the AST cache between requests - java.nio.file.Files.writeString(java.nio.file.Path.of(URI.create(uri)), contents) - when: 'format the document so comment metadata is re-derived' - open(service, uri, contents) + openOnDisk(service, uri, contents) def edits = service.formatting(URI.create(uri), new FormattingOptions(4, true)) - then: - edits.first().getNewText() == contents + then: 'the document is already canonical, so formatting returns no edits' + edits.isEmpty() when: 'convert the same cached AST to static types' def response = service.executeCommand('nextflow.server.convertScriptToTyped', [asJson(uri)], LanguageServerConfiguration.defaults()) @@ -269,7 +265,7 @@ class ConvertScriptStaticTypesTest extends Specification { }'''.stripIndent() cleanup: - java.nio.file.Files.deleteIfExists(java.nio.file.Path.of(URI.create(uri))) + deleteOnDisk(uri) } def 'should convert tuple outputs' () { diff --git a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy index 0b6f4d6..9253d3d 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ScriptFormattingTest.groovy @@ -16,9 +16,6 @@ package nextflow.lsp.services.script -import java.nio.file.Files -import java.nio.file.Path - import nextflow.script.formatter.FormattingOptions import spock.lang.Specification @@ -33,7 +30,9 @@ class ScriptFormattingTest extends Specification { boolean checkFormat(ScriptService service, String uri, FormattingOptions options, String before, String after) { open(service, uri, before) def textEdits = service.formatting(URI.create(uri), options) - assert textEdits.first().getNewText() == after.stripIndent() + // no edits means the document was already formatted + def newText = textEdits ? textEdits.first().getNewText() : before.stripIndent() + assert newText == after.stripIndent() return true } @@ -41,6 +40,14 @@ class ScriptFormattingTest extends Specification { return checkFormat(service, uri, new FormattingOptions(4, true), before, after) } + boolean checkRoundTrip(ScriptService service, String uri, FormattingOptions options, String source) { + return checkFormat(service, uri, options, source, source) + } + + boolean checkRoundTrip(ScriptService service, String uri, String source) { + return checkFormat(service, uri, source, source) + } + def 'should format a script' () { given: def service = getScriptService() @@ -189,15 +196,7 @@ class ScriptFormattingTest extends Specification { expect: // trailing comments, dangling comments at the end of a block and at // the end of the file are all preserved - checkFormat(service, uri, - '''\ - workflow { - x = 1 // trailing comment - // comment after the last statement - } - - // comment at the end of the file - ''', + checkRoundTrip(service, uri, '''\ workflow { x = 1 // trailing comment @@ -208,22 +207,7 @@ class ScriptFormattingTest extends Specification { ''' ) // a commented-out process is preserved - checkFormat(service, uri, - '''\ - // process FOO { - // script: - // "true" - // } - - process BAR { - script: - "true" - } - - workflow { - BAR() - } - ''', + checkRoundTrip(service, uri, '''\ // process FOO { // script: @@ -275,12 +259,7 @@ class ScriptFormattingTest extends Specification { def options = new FormattingOptions(4, true, false, false, false, 0) expect: - checkFormat(service, uri, options, - '''\ - workflow { - ALIGN_AND_SORT(samples_channel, reference_genome, annotation_file, params.threads, extra_arg_one, extra_arg_two, extra_arg_three) - } - ''', + checkRoundTrip(service, uri, options, '''\ workflow { ALIGN_AND_SORT(samples_channel, reference_genome, annotation_file, params.threads, extra_arg_one, extra_arg_two, extra_arg_three) @@ -310,20 +289,7 @@ class ScriptFormattingTest extends Specification { ''' ) // a fmt: off / fmt: on region round-trips unchanged - checkFormat(service, uri, - '''\ - workflow { - a = 1 - - // fmt: off - matrix = [ - [1, 0], - [0, 1] ] - // fmt: on - - b = 2 - } - ''', + checkRoundTrip(service, uri, '''\ workflow { a = 1 @@ -416,7 +382,8 @@ class ScriptFormattingTest extends Specification { // formatting the same document repeatedly without a document change // re-derives the comment metadata on the same cached AST -- the // output must not change, including for comments inside wrapped - // expressions + // expressions; the input is deliberately non-canonical so that + // every request returns an edit def service = getScriptService() def uri = getUri('main.nf') def contents = '''\ @@ -430,23 +397,23 @@ class ScriptFormattingTest extends Specification { // comment on chain link .map { x -> x } .view() + y=1 } '''.stripIndent() - // the file must exist on disk so that the deferred workspace scan - // does not evict it from the AST cache between formatting calls - Files.writeString(Path.of(URI.create(uri)), contents) + def expected = contents.replace('y=1', 'y = 1') when: - open(service, uri, contents) - def edits = (1..3).collect { - service.formatting(URI.create(uri), new FormattingOptions(4, true)) + 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: - edits.every { it.first().getNewText() == contents } + texts == [expected] * 3 cleanup: - Files.deleteIfExists(Path.of(URI.create(uri))) + deleteOnDisk(uri) } }