Skip to content

Commit 432fce8

Browse files
bentshermanbentsherman
committed
Perform type checking only for scripts that enable static typing
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
1 parent dd841fd commit 432fce8

9 files changed

Lines changed: 32 additions & 76 deletions

File tree

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -462,8 +462,7 @@ public void didChangeConfiguration(DidChangeConfigurationParams params) {
462462
withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.maheshForm"), configuration.maheshForm()),
463463
withDefault(JsonUtils.getInteger(settings, "nextflow.completion.maxItems"), configuration.maxCompletionItems()),
464464
withDefault(JsonUtils.getString(settings, "nextflow.pluginRegistryUrl"), configuration.pluginRegistryUrl()),
465-
withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.sortDeclarations"), configuration.sortDeclarations()),
466-
withDefault(JsonUtils.getBoolean(settings, "nextflow.typeChecking"), configuration.typeChecking())
465+
withDefault(JsonUtils.getBoolean(settings, "nextflow.formatting.sortDeclarations"), configuration.sortDeclarations())
467466
);
468467

469468
if( shouldInitialize(oldConfiguration, configuration) )
@@ -484,8 +483,7 @@ private <T> T withDefault(T value, T defaultValue) {
484483
private boolean shouldInitialize(LanguageServerConfiguration previous, LanguageServerConfiguration current) {
485484
return previous.errorReportingMode() != current.errorReportingMode()
486485
|| !DefaultGroovyMethods.equals(previous.excludePatterns(), current.excludePatterns())
487-
|| previous.pluginRegistryUrl() != current.pluginRegistryUrl()
488-
|| previous.typeChecking() != current.typeChecking();
486+
|| previous.pluginRegistryUrl() != current.pluginRegistryUrl();
489487
}
490488

491489
private void initializeWorkspaces() {

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,7 @@ public record LanguageServerConfiguration(
2828
boolean maheshForm,
2929
int maxCompletionItems,
3030
String pluginRegistryUrl,
31-
boolean sortDeclarations,
32-
boolean typeChecking
31+
boolean sortDeclarations
3332
) {
3433

3534
public static LanguageServerConfiguration defaults() {
@@ -43,7 +42,6 @@ public static LanguageServerConfiguration defaults() {
4342
false,
4443
100,
4544
"https://registry.nextflow.io/api/",
46-
false,
4745
false
4846
);
4947
}

‎src/main/java/nextflow/lsp/services/config/ConfigAstCache.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ protected Set<URI> analyze(Set<URI> uris, FileCache fileCache) {
9595
continue;
9696
// phase 3: name checking
9797
new ConfigResolveVisitor(sourceUnit, compiler().compilationUnit(), Types.DEFAULT_CONFIG_IMPORTS).visit();
98-
new ConfigSpecVisitor(sourceUnit, pluginSpecCache, configuration.typeChecking()).visit();
98+
new ConfigSpecVisitor(sourceUnit, pluginSpecCache).visit();
9999
}
100100

101101
return changedUris;

‎src/main/java/nextflow/lsp/services/config/ConfigSpecVisitor.java‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,14 +66,11 @@ public class ConfigSpecVisitor extends ConfigVisitorSupport {
6666

6767
private PluginSpecCache pluginSpecCache;
6868

69-
private boolean typeChecking;
70-
7169
private Stack<String> scopes = new Stack<>();
7270

73-
public ConfigSpecVisitor(SourceUnit sourceUnit, PluginSpecCache pluginSpecCache, boolean typeChecking) {
71+
public ConfigSpecVisitor(SourceUnit sourceUnit, PluginSpecCache pluginSpecCache) {
7472
this.sourceUnit = sourceUnit;
7573
this.pluginSpecCache = pluginSpecCache;
76-
this.typeChecking = typeChecking;
7774
}
7875

7976
@Override
@@ -170,8 +167,6 @@ public void visitConfigAssign(ConfigAssignNode node) {
170167
return;
171168
}
172169
// validate type
173-
if( !typeChecking )
174-
return;
175170
var expectedTypes = option.types().stream()
176171
.map(t -> ClassHelper.makeCached(t).getPlainNodeReference())
177172
.toList();
@@ -188,7 +183,7 @@ public void visitConfigAssign(ConfigAssignNode node) {
188183
}
189184

190185
private ClassNode inferredType(Expression node, List<String> scopes) {
191-
new TypeCheckingVisitorEx(sourceUnit, true).visit(node);
186+
new TypeCheckingVisitorEx(sourceUnit).visit(node);
192187
var type = TypeCheckingUtils.getType(node);
193188
if( node instanceof ClosureExpression ce && "process".equals(scopes.get(0)) ) {
194189
var visitor = new ReturnStatementVisitor(sourceUnit);

‎src/main/java/nextflow/lsp/services/script/ScriptAstCache.java‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import nextflow.script.control.Phases;
4343
import nextflow.script.control.ResolveIncludeVisitor;
4444
import nextflow.script.control.ScriptResolveVisitor;
45+
import nextflow.script.control.TypeCheckingVisitor;
4546
import nextflow.script.control.TypeCheckingVisitorEx;
4647
import nextflow.script.parser.ScriptParserPluginFactory;
4748
import nextflow.script.types.Types;
@@ -62,8 +63,6 @@ public class ScriptAstCache extends ASTNodeCache {
6263

6364
private GroovyLibCache libCache;
6465

65-
private LanguageServerConfiguration configuration;
66-
6766
private PluginSpecCache pluginSpecCache;
6867

6968
public ScriptAstCache(String rootUri) {
@@ -97,7 +96,6 @@ private static CompilerConfiguration createConfiguration() {
9796
}
9897

9998
public void initialize(LanguageServerConfiguration configuration, PluginSpecCache pluginSpecCache) {
100-
this.configuration = configuration;
10199
this.pluginSpecCache = pluginSpecCache;
102100
}
103101

@@ -145,7 +143,12 @@ protected Set<URI> analyze(Set<URI> uris, FileCache fileCache) {
145143
if( sourceUnit == null || sourceUnit.getErrorCollector().hasErrors() )
146144
continue;
147145
// phase 4: type checking
148-
new TypeCheckingVisitorEx(sourceUnit, configuration.typeChecking()).visit();
146+
if( sourceUnit.getAST() instanceof ScriptNode sn ) {
147+
if( sn.isTypingEnabled() )
148+
new TypeCheckingVisitorEx(sourceUnit).visit();
149+
else
150+
new TypeCheckingVisitor(sourceUnit).visit();
151+
}
149152
}
150153

151154
return changedUris;

‎src/main/java/nextflow/script/control/TypeCheckingVisitorEx.java‎

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,8 @@ public class TypeCheckingVisitorEx extends ScriptVisitorSupport {
103103

104104
private SourceUnit sourceUnit;
105105

106-
private boolean experimental;
107-
108-
public TypeCheckingVisitorEx(SourceUnit sourceUnit, boolean experimental) {
106+
public TypeCheckingVisitorEx(SourceUnit sourceUnit) {
109107
this.sourceUnit = sourceUnit;
110-
this.experimental = experimental;
111108
}
112109

113110
@Override
@@ -137,8 +134,6 @@ public void visit() {
137134

138135
@Override
139136
public void visitFeatureFlag(FeatureFlagNode node) {
140-
if( !experimental )
141-
return;
142137
var fn = node.target;
143138
if( fn == null )
144139
return;
@@ -157,8 +152,6 @@ public void visitParam(Parameter node) {
157152
}
158153

159154
private void visitParameter(Parameter node, boolean allowPathCoercion) {
160-
if( !experimental )
161-
return;
162155
if( !node.hasInitialExpression() )
163156
return;
164157
var expectedType = node.getType();
@@ -174,11 +167,6 @@ private void visitParameter(Parameter node, boolean allowPathCoercion) {
174167

175168
@Override
176169
public void visitWorkflow(WorkflowNode node) {
177-
if( !experimental ) {
178-
super.visitWorkflow(node);
179-
return;
180-
}
181-
182170
currentWorkflow = node;
183171

184172
visit(node.main);
@@ -288,9 +276,6 @@ public void visitFunction(FunctionNode node) {
288276
visit(node.getCode());
289277

290278
// check return statements against declared return type
291-
if( !experimental )
292-
return;
293-
294279
var visitor = new ReturnStatementVisitor(sourceUnit);
295280
visitor.visit(node.getReturnType(), node.getCode());
296281

@@ -301,10 +286,6 @@ public void visitFunction(FunctionNode node) {
301286

302287
@Override
303288
public void visitOutput(OutputNode node) {
304-
if( !experimental ) {
305-
super.visitOutput(node);
306-
return;
307-
}
308289
var type = node.getType();
309290
var elementType = dataflowElementType(type);
310291
for( var stmt : asBlockStatements(node.body) ) {
@@ -390,8 +371,6 @@ public void visitDeclarationExpression(DeclarationExpression node) {
390371
}
391372

392373
private void applyAssignment(BinaryExpression node) {
393-
if( !experimental )
394-
return;
395374
var target = node.getLeftExpression();
396375
var source = node.getRightExpression();
397376
if( source instanceof EmptyExpression )
@@ -432,11 +411,6 @@ private void applyTupleAssignment(TupleExpression target, ClassNode sourceType)
432411

433412
@Override
434413
public void visitMethodCallExpression(MethodCallExpression node) {
435-
if( !experimental ) {
436-
super.visitMethodCallExpression(node);
437-
return;
438-
}
439-
440414
// resolve argument types (except for closures)
441415
var arguments = asMethodCallArguments(node);
442416
boolean hasClosureArgs = false;
@@ -505,10 +479,10 @@ else if( node.getNodeMetaData(ASTNodeMarker.METHOD_TARGET) instanceof MethodNode
505479
node.putNodeMetaData(ASTNodeMarker.METHOD_TARGET, dummyMethod);
506480
}
507481
}
508-
else if( experimental && node.getNodeMetaData(ASTNodeMarker.METHOD_OVERLOADS) != null ) {
482+
else if( node.getNodeMetaData(ASTNodeMarker.METHOD_OVERLOADS) != null ) {
509483
addSoftError(String.format("Function `%s` (with multiple signatures) was called with incorrect number of arguments and/or incorrect argument types", node.getMethodAsString()), node.getMethod());
510484
}
511-
else if( experimental && !node.isImplicitThis() ) {
485+
else if( !node.isImplicitThis() ) {
512486
var className = className(receiver);
513487
addError(String.format("Unrecognized method `%s` for %s", node.getMethodAsString(), className), node.getMethod());
514488
}
@@ -837,9 +811,6 @@ private boolean checkTupleCall(MethodCallExpression node) {
837811
public void visitBinaryExpression(BinaryExpression node) {
838812
super.visitBinaryExpression(node);
839813

840-
if( !experimental )
841-
return;
842-
843814
var op = node.getOperation();
844815
if( op.getType() == Types.PLUS && checkRecordSum(node) )
845816
return;
@@ -1019,9 +990,6 @@ public void visitShortTernaryExpression(ElvisOperatorExpression node) {
1019990
}
1020991

1021992
private void applyConditionalExpression(TernaryExpression node) {
1022-
if( !experimental )
1023-
return;
1024-
1025993
var trueExpr = node.getTrueExpression();
1026994
var falseExpr = node.getFalseExpression();
1027995
var trueType = getType(trueExpr);
@@ -1066,8 +1034,6 @@ public void visitClosureExpression(ClosureExpression node) {
10661034
super.visitClosureExpression(node);
10671035

10681036
// resolve return type and check against declared return type
1069-
if( !experimental )
1070-
return;
10711037
var returnType = (ClassNode) node.getNodeMetaData(ASTNodeMarker.INFERRED_RETURN_TYPE);
10721038
if( returnType != null ) {
10731039
var visitor = new ReturnStatementVisitor(sourceUnit);
@@ -1083,9 +1049,6 @@ public void visitClosureExpression(ClosureExpression node) {
10831049
public void visitListExpression(ListExpression node) {
10841050
super.visitListExpression(node);
10851051

1086-
if( !experimental )
1087-
return;
1088-
10891052
ClassNode elementType = null;
10901053
for( var el : node.getExpressions() ) {
10911054
var type = getType(el);
@@ -1114,9 +1077,6 @@ public void visitMapExpression(MapExpression node) {
11141077
public void visitRangeExpression(RangeExpression node) {
11151078
super.visitRangeExpression(node);
11161079

1117-
if( !experimental )
1118-
return;
1119-
11201080
var lhs = node.getFrom();
11211081
var rhs = node.getTo();
11221082
var lhsType = getType(lhs);
@@ -1147,24 +1107,18 @@ public void visitRangeExpression(RangeExpression node) {
11471107
@Override
11481108
public void visitUnaryMinusExpression(UnaryMinusExpression node) {
11491109
super.visitUnaryMinusExpression(node);
1150-
if( !experimental )
1151-
return;
11521110
resolveUnaryOpOrFail(node.getExpression(), "-", "negative", node);
11531111
}
11541112

11551113
@Override
11561114
public void visitUnaryPlusExpression(UnaryPlusExpression node) {
11571115
super.visitUnaryPlusExpression(node);
1158-
if( !experimental )
1159-
return;
11601116
resolveUnaryOpOrFail(node.getExpression(), "+", "positive", node);
11611117
}
11621118

11631119
@Override
11641120
public void visitBitwiseNegationExpression(BitwiseNegationExpression node) {
11651121
super.visitBitwiseNegationExpression(node);
1166-
if( !experimental )
1167-
return;
11681122
resolveUnaryOpOrFail(node.getExpression(), "~", "bitwiseNegate", node);
11691123
}
11701124

@@ -1181,8 +1135,6 @@ private void resolveUnaryOpOrFail(Expression operand, String op, String method,
11811135
@Override
11821136
public void visitCastExpression(CastExpression node) {
11831137
super.visitCastExpression(node);
1184-
if( !experimental )
1185-
return;
11861138
var sourceType = getType(node.getExpression());
11871139
var targetType = node.getType();
11881140
if( ClassHelper.isObjectType(sourceType) || TypesEx.isAssignableFrom(targetType, sourceType) )
@@ -1219,9 +1171,6 @@ private boolean checkRecordCast(ClassNode targetType, ClassNode sourceType, ASTN
12191171
public void visitPropertyExpression(PropertyExpression node) {
12201172
super.visitPropertyExpression(node);
12211173

1222-
if( !experimental )
1223-
return;
1224-
12251174
var receiver = node.getObjectExpression();
12261175
var receiverType = getType(receiver);
12271176
if( ClassHelper.isDynamicTyped(receiverType) )

‎src/test/groovy/nextflow/lsp/services/config/ConfigSpecTest.groovy‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class ConfigSpecTest extends Specification {
4848
def pluginSpecCache = new PluginSpecCache(configuration.pluginRegistryUrl())
4949
def source = configParser.parse('nextflow.config', contents.stripIndent())
5050
new ConfigResolveVisitor(source, configParser.compiler().compilationUnit(), Types.DEFAULT_CONFIG_IMPORTS).visit()
51-
new ConfigSpecVisitor(source, pluginSpecCache, true).visit()
51+
new ConfigSpecVisitor(source, pluginSpecCache).visit()
5252
return source
5353
}
5454

‎src/test/groovy/nextflow/script/types/TypeCheckingTest.groovy‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717
package nextflow.script.types
1818

1919
import nextflow.script.ast.ASTNodeMarker
20+
import nextflow.script.ast.FeatureFlagNode
2021
import nextflow.script.control.ScriptParser
2122
import nextflow.script.control.ScriptResolveVisitor
2223
import nextflow.script.control.TypeCheckingVisitorEx
2324
import org.codehaus.groovy.ast.ASTNode
2425
import org.codehaus.groovy.ast.ClassHelper
26+
import org.codehaus.groovy.ast.expr.ConstantExpression
2527
import org.codehaus.groovy.ast.expr.Expression
2628
import org.codehaus.groovy.ast.stmt.BlockStatement
2729
import org.codehaus.groovy.ast.stmt.ExpressionStatement
@@ -51,8 +53,9 @@ class TypeCheckingTest extends Specification {
5153

5254
SourceUnit parse(String contents) {
5355
def source = scriptParser.parse('main.nf', contents.stripIndent())
56+
source.getAST()?.addFeatureFlag(new FeatureFlagNode("nextflow.preview.types", new ConstantExpression(true)))
5457
new ScriptResolveVisitor(source, scriptParser.compiler().compilationUnit(), Types.DEFAULT_SCRIPT_IMPORTS, Collections.emptyList()).visit()
55-
new TypeCheckingVisitorEx(source, true).visit()
58+
new TypeCheckingVisitorEx(source).visit()
5659
return source
5760
}
5861

@@ -142,6 +145,8 @@ class TypeCheckingTest extends Specification {
142145
when:
143146
def errors = getErrors(
144147
'''\
148+
nextflow.preview.types = true
149+
145150
workflow hello {
146151
emit:
147152
result: String = 42
@@ -150,13 +155,15 @@ class TypeCheckingTest extends Specification {
150155
)
151156
then:
152157
errors.size() == 1
153-
errors[0].getStartLine() == 3
158+
errors[0].getStartLine() == 5
154159
errors[0].getStartColumn() == 5
155160
errors[0].getOriginalMessage() == "Assignment target with type String cannot be assigned to value with type Integer"
156161

157162
when:
158163
errors = getErrors(
159164
'''\
165+
nextflow.preview.types = true
166+
160167
workflow hello {
161168
emit:
162169
result: Integer = 42
@@ -633,6 +640,8 @@ class TypeCheckingTest extends Specification {
633640
expect:
634641
check(
635642
'''
643+
nextflow.preview.types = true
644+
636645
workflow hello {
637646
take:
638647
messages: Channel<String>
@@ -653,6 +662,8 @@ class TypeCheckingTest extends Specification {
653662
when:
654663
def exp = parseExpression(
655664
'''\
665+
nextflow.preview.types = true
666+
656667
workflow hello {
657668
emit:
658669
result: Integer = 42

‎src/test/groovy/nextflow/script/types/TypeCheckingUtilsTest.groovy‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,8 @@ class TypeCheckingUtilsTest extends Specification {
154154
when:
155155
def exp = parseExpression(
156156
'''
157+
nextflow.preview.types = true
158+
157159
workflow hello {
158160
take:
159161
target: String

0 commit comments

Comments
 (0)