From 3f268e3b845891f38c9843c8a7d2bc1f1dad58b0 Mon Sep 17 00:00:00 2001 From: Ben Sherman Date: Thu, 23 Jul 2026 17:57:28 -0500 Subject: [PATCH] Re-check consumer scripts when an included module changes (#165) Editing a module left stale diagnostics in its consumers. The consumer was re-type-checked, but only the directly-changed files were re-parsed, so the consumer's nodes kept stale INFERRED_TYPE metadata and getType() short-circuited on the cached value -- a changed process output type was never observed. Transitive consumers were not re-checked at all. Expand the invalidation set (before parsing) to include every cached file that transitively includes a changed file, so consumers are re-parsed from fresh AST nodes and their cross-file inferred types are recomputed. Signed-off-by: Ben Sherman --- .../java/nextflow/lsp/ast/ASTNodeCache.java | 15 ++++ .../lsp/services/script/ScriptAstCache.java | 36 +++++++++ .../script/ScriptDiagnosticsTest.groovy | 73 +++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/src/main/java/nextflow/lsp/ast/ASTNodeCache.java b/src/main/java/nextflow/lsp/ast/ASTNodeCache.java index bfd44341..45425a43 100644 --- a/src/main/java/nextflow/lsp/ast/ASTNodeCache.java +++ b/src/main/java/nextflow/lsp/ast/ASTNodeCache.java @@ -88,6 +88,10 @@ public void clear() { * @param fileCache */ public Set update(Set uris, FileCache fileCache) { + // expand the invalidation set to include dependents (e.g. files that + // include a changed module) so their cached analysis is recomputed + uris = expandInvalidation(uris); + // invalidate cache for each source file for( var uri : uris ) { var nodes = nodesByURI.remove(uri); @@ -158,6 +162,17 @@ public Set update(Set uris, FileCache fileCache) { return result; } + /** + * Expand a set of changed files to include any cached files whose analysis + * depends on them (e.g. files that include a changed module). The default + * implementation returns the set unchanged. + * + * @param uris + */ + protected Set expandInvalidation(Set uris) { + return uris; + } + /** * Perform additional AST analysis for a set of source files. * Return the set of files whose errors have changed. diff --git a/src/main/java/nextflow/lsp/services/script/ScriptAstCache.java b/src/main/java/nextflow/lsp/services/script/ScriptAstCache.java index dd7465c6..6213d7ed 100644 --- a/src/main/java/nextflow/lsp/services/script/ScriptAstCache.java +++ b/src/main/java/nextflow/lsp/services/script/ScriptAstCache.java @@ -18,8 +18,10 @@ import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.HashMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -157,6 +159,40 @@ protected Set analyze(Set uris, FileCache fileCache) { return changedUris; } + /** + * Expand a set of changed files to include every cached file that + * (transitively) includes one of them. Those consumers must be re-parsed + * so that their cross-file inferred types (e.g. a process's record output) + * are recomputed from fresh AST nodes rather than stale cached metadata. + * + * @param uris + */ + @Override + protected Set expandInvalidation(Set uris) { + // reverse include graph: module uri -> files that include it + var dependents = new HashMap>(); + for( var uri : getUris() ) { + for( var include : getIncludeNodes(uri) ) { + var depUri = localIncludeUri(uri, include.source.getText()); + if( depUri != null ) + dependents.computeIfAbsent(depUri, (k) -> new HashSet<>()).add(uri); + } + } + if( dependents.isEmpty() ) + return uris; + + var result = new HashSet<>(uris); + var queue = new ArrayDeque<>(uris); + while( !queue.isEmpty() ) { + var uri = queue.poll(); + for( var consumer : dependents.getOrDefault(uri, Collections.emptySet()) ) { + if( result.add(consumer) ) + queue.add(consumer); + } + } + return result; + } + /** * Order the given URIs so that a file appears after every module it * includes (dependencies first), via depth-first post-order traversal. diff --git a/src/test/groovy/nextflow/lsp/services/script/ScriptDiagnosticsTest.groovy b/src/test/groovy/nextflow/lsp/services/script/ScriptDiagnosticsTest.groovy index 220c3ff1..7001cac5 100644 --- a/src/test/groovy/nextflow/lsp/services/script/ScriptDiagnosticsTest.groovy +++ b/src/test/groovy/nextflow/lsp/services/script/ScriptDiagnosticsTest.groovy @@ -144,6 +144,79 @@ class ScriptDiagnosticsTest extends Specification { diagnostics.findAll { it.message.contains('is not compatible with process input') } == [] } + def 'should re-check a consumer when an included module changes' () { + given: + def client = new TestLanguageClient() + def service = getScriptService(client) + def mainUri = getUri('workflow.nf') + def producerUri = getUri('producer.nf') + def consumerUri = getUri('consumer.nf') + + def producer = { String outputExtra -> """\ + nextflow.enable.types = true + + process PRODUCER { + input: + record(id: String) + + output: + record(id: id${outputExtra}) + + script: + \"\"\" + echo hello > data.txt + \"\"\" + } + """ } + def consumer = '''\ + nextflow.enable.types = true + + process CONSUMER { + input: + record(id: String, data: Path) + + output: + record(id: id) + + script: + """ + cat ${data} + """ + } + ''' + def main = '''\ + nextflow.enable.types = true + + include { PRODUCER } from './producer.nf' + include { CONSUMER } from './consumer.nf' + + workflow { + ch_produced = PRODUCER(channel.of(record(id: 'sample1'))) + CONSUMER(ch_produced) + } + ''' + + when: 'producer emits a compatible record { id, data }' + open(service, producerUri, producer(", data: file('data.txt')")) + open(service, consumerUri, consumer) + open(service, mainUri, main) + service.updateNow() + then: + client.getDiagnostics(mainUri).findAll { it.message.contains('is not compatible with process input') } == [] + + when: 'only the producer module is edited to emit an incompatible record { id }' + open(service, producerUri, producer('')) + service.updateNow() + then: 'the consumer call is re-checked and now reports the mismatch' + client.getDiagnostics(mainUri).findAll { it.message.contains('is not compatible with process input') }.size() == 1 + + when: 'the producer module is restored' + open(service, producerUri, producer(", data: file('data.txt')")) + service.updateNow() + then: 'the stale warning is cleared' + client.getDiagnostics(mainUri).findAll { it.message.contains('is not compatible with process input') } == [] + } + def 'should clear diagnostics when an error is fixed' () { given: def client = new TestLanguageClient()