Skip to content

Commit cbd7b4e

Browse files
bentshermanbentsherman
committed
Improve type checking in config files
1 parent f705740 commit cbd7b4e

File tree

5 files changed

+421
-66
lines changed

5 files changed

+421
-66
lines changed

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,10 +96,6 @@ protected Set<URI> analyze(Set<URI> uris, FileCache fileCache) {
9696
// phase 3: name checking
9797
new ConfigResolveVisitor(sourceUnit, compiler().compilationUnit(), Types.DEFAULT_CONFIG_IMPORTS).visit();
9898
new ConfigSpecVisitor(sourceUnit, pluginSpecCache, configuration.typeChecking()).visit();
99-
if( sourceUnit.getErrorCollector().hasErrors() )
100-
continue;
101-
// phase 4: type checking
102-
// TODO
10399
}
104100

105101
return changedUris;

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,16 @@
3333
import nextflow.lsp.spec.PluginSpecCache;
3434
import nextflow.script.control.PhaseAware;
3535
import nextflow.script.control.Phases;
36+
import nextflow.script.control.ReturnStatementVisitor;
37+
import nextflow.script.control.TypeCheckingVisitorEx;
38+
import nextflow.script.types.TypeCheckingUtils;
3639
import nextflow.script.types.TypesEx;
3740
import org.codehaus.groovy.ast.ASTNode;
3841
import org.codehaus.groovy.ast.ClassHelper;
3942
import org.codehaus.groovy.ast.ClassNode;
43+
import org.codehaus.groovy.ast.expr.ClosureExpression;
4044
import org.codehaus.groovy.ast.expr.ConstantExpression;
45+
import org.codehaus.groovy.ast.expr.Expression;
4146
import org.codehaus.groovy.control.SourceUnit;
4247
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
4348
import org.codehaus.groovy.control.messages.WarningMessage;
@@ -169,16 +174,30 @@ public void visitConfigAssign(ConfigAssignNode node) {
169174
var expectedTypes = option.types().stream()
170175
.map(t -> ClassHelper.makeCached(t).getPlainNodeReference())
171176
.toList();
172-
var actualType = node.value.getType();
177+
var actualType = inferredType(node.value, names);
173178
if( !isAssignableFromAny(expectedTypes, actualType) ) {
174179
var validTypes = expectedTypes.stream()
175180
.map(cn -> TypesEx.getName(cn))
176181
.collect(Collectors.joining(", "));
177-
var message = "Config option '" + fqName + "' with cannot be assigned to value with type " + TypesEx.getName(actualType) + " -- valid types are: " + validTypes;
182+
var message = expectedTypes.size() == 1
183+
? "Config option '" + fqName + "' with type " + TypesEx.getName(expectedTypes.get(0)) + " cannot be assigned to value with type " + TypesEx.getName(actualType)
184+
: "Config option '" + fqName + "' cannot be assigned to value with type " + TypesEx.getName(actualType) + " -- valid types are: " + validTypes;
178185
addWarning(message, String.join(".", node.names), node.getLineNumber(), node.getColumnNumber());
179186
}
180187
}
181188

189+
private ClassNode inferredType(Expression node, List<String> scopes) {
190+
new TypeCheckingVisitorEx(sourceUnit, true).visit(node);
191+
var type = TypeCheckingUtils.getType(node);
192+
if( node instanceof ClosureExpression ce && "process".equals(scopes.get(0)) ) {
193+
var visitor = new ReturnStatementVisitor(sourceUnit);
194+
visitor.visit(ClassHelper.dynamicType(), ce.getCode());
195+
var inferredReturnType = visitor.getInferredReturnType();
196+
return inferredReturnType != null ? inferredReturnType : ClassHelper.dynamicType();
197+
}
198+
return type;
199+
}
200+
182201
private boolean isAssignableFromAny(List<ClassNode> targetTypes, ClassNode sourceType) {
183202
if( targetTypes.isEmpty() )
184203
return true;

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

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
/*
2+
* Copyright 2024-2025, Seqera Labs
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package nextflow.script.control;
17+
18+
import java.util.ArrayList;
19+
20+
import nextflow.script.types.TypesEx;
21+
import org.codehaus.groovy.ast.ASTNode;
22+
import org.codehaus.groovy.ast.ClassHelper;
23+
import org.codehaus.groovy.ast.ClassNode;
24+
import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
25+
import org.codehaus.groovy.ast.stmt.BlockStatement;
26+
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
27+
import org.codehaus.groovy.ast.stmt.IfStatement;
28+
import org.codehaus.groovy.ast.stmt.ReturnStatement;
29+
import org.codehaus.groovy.ast.stmt.Statement;
30+
import org.codehaus.groovy.control.SourceUnit;
31+
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
32+
import org.codehaus.groovy.syntax.SyntaxException;
33+
34+
import static nextflow.script.ast.ASTUtils.*;
35+
import static nextflow.script.types.TypeCheckingUtils.*;
36+
37+
/**
38+
* Infer the return type of a code block based on implicit
39+
* and explicit return statements.
40+
*
41+
* @see org.codehaus.groovy.classgen.ReturnAdder
42+
*
43+
* @author Ben Sherman <bentshermann@gmail.com>
44+
*/
45+
public class ReturnStatementVisitor extends ClassCodeVisitorSupport {
46+
47+
private SourceUnit sourceUnit;
48+
49+
private ClassNode returnType;
50+
51+
private ClassNode inferredReturnType;
52+
53+
public ReturnStatementVisitor(SourceUnit sourceUnit) {
54+
this.sourceUnit = sourceUnit;
55+
}
56+
57+
@Override
58+
protected SourceUnit getSourceUnit() {
59+
return sourceUnit;
60+
}
61+
62+
public void visit(ClassNode returnType, Statement code) {
63+
this.returnType = returnType;
64+
visit(addReturnsIfNeeded(code));
65+
this.returnType = null;
66+
}
67+
68+
private Statement addReturnsIfNeeded(Statement node) {
69+
if( node instanceof BlockStatement block && !block.isEmpty() ) {
70+
var statements = new ArrayList<>(block.getStatements());
71+
int lastIndex = statements.size() - 1;
72+
var last = addReturnsIfNeeded(statements.get(lastIndex));
73+
statements.set(lastIndex, last);
74+
return new BlockStatement(statements, block.getVariableScope());
75+
}
76+
77+
if( node instanceof ExpressionStatement es ) {
78+
return new ReturnStatement(es.getExpression());
79+
}
80+
81+
if( node instanceof IfStatement ies ) {
82+
return new IfStatement(
83+
ies.getBooleanExpression(),
84+
addReturnsIfNeeded(ies.getIfBlock()),
85+
addReturnsIfNeeded(ies.getElseBlock()) );
86+
}
87+
88+
return node;
89+
}
90+
91+
@Override
92+
public void visitReturnStatement(ReturnStatement node) {
93+
var sourceType = getType(node.getExpression());
94+
if( inferredReturnType != null && !ClassHelper.isDynamicTyped(returnType) ) {
95+
if( !TypesEx.isAssignableFrom(inferredReturnType, sourceType) )
96+
addError(String.format("Return value with type %s does not match previous return type (%s)", TypesEx.getName(sourceType), TypesEx.getName(inferredReturnType)), node);
97+
}
98+
else if( TypesEx.isAssignableFrom(returnType, sourceType) ) {
99+
inferredReturnType = sourceType;
100+
}
101+
else {
102+
addError(String.format("Return value with type %s does not match the declared return type (%s)", TypesEx.getName(sourceType), TypesEx.getName(returnType)), node);
103+
}
104+
}
105+
106+
public ClassNode getInferredReturnType() {
107+
return inferredReturnType;
108+
}
109+
110+
@Override
111+
public void addError(String message, ASTNode node) {
112+
var cause = new TypeError(message, node);
113+
var errorMessage = new SyntaxErrorMessage(cause, sourceUnit);
114+
sourceUnit.getErrorCollector().addErrorAndContinue(errorMessage);
115+
}
116+
117+
private class TypeError extends SyntaxException implements PhaseAware {
118+
119+
public TypeError(String message, ASTNode node) {
120+
super(message, node);
121+
}
122+
123+
@Override
124+
public int getPhase() {
125+
return Phases.TYPE_CHECKING;
126+
}
127+
}
128+
}

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

Lines changed: 2 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.codehaus.groovy.ast.ASTNode;
4747
import org.codehaus.groovy.ast.ClassHelper;
4848
import org.codehaus.groovy.ast.ClassNode;
49-
import org.codehaus.groovy.ast.CodeVisitorSupport;
5049
import org.codehaus.groovy.ast.FieldNode;
5150
import org.codehaus.groovy.ast.GenericsType;
5251
import org.codehaus.groovy.ast.MethodNode;
@@ -75,8 +74,6 @@
7574
import org.codehaus.groovy.ast.expr.VariableExpression;
7675
import org.codehaus.groovy.ast.stmt.BlockStatement;
7776
import org.codehaus.groovy.ast.stmt.ExpressionStatement;
78-
import org.codehaus.groovy.ast.stmt.IfStatement;
79-
import org.codehaus.groovy.ast.stmt.ReturnStatement;
8077
import org.codehaus.groovy.ast.stmt.Statement;
8178
import org.codehaus.groovy.control.SourceUnit;
8279
import org.codehaus.groovy.control.messages.SyntaxErrorMessage;
@@ -282,69 +279,14 @@ public void visitFunction(FunctionNode node) {
282279
if( !experimental )
283280
return;
284281

285-
var visitor = new ReturnStatementVisitor();
282+
var visitor = new ReturnStatementVisitor(sourceUnit);
286283
visitor.visit(node.getReturnType(), node.getCode());
287284

288285
var inferredReturnType = visitor.getInferredReturnType();
289286
if( inferredReturnType != null && ClassHelper.isDynamicTyped(node.getReturnType()) )
290287
node.setReturnType(inferredReturnType);
291288
}
292289

293-
private class ReturnStatementVisitor extends CodeVisitorSupport {
294-
295-
private ClassNode returnType;
296-
297-
private ClassNode inferredReturnType;
298-
299-
public void visit(ClassNode returnType, Statement code) {
300-
this.returnType = returnType;
301-
visit(addReturnsIfNeeded(code));
302-
this.returnType = null;
303-
}
304-
305-
private Statement addReturnsIfNeeded(Statement node) {
306-
if( node instanceof BlockStatement block && !block.isEmpty() ) {
307-
var statements = new ArrayList<>(block.getStatements());
308-
int lastIndex = statements.size() - 1;
309-
var last = addReturnsIfNeeded(statements.get(lastIndex));
310-
statements.set(lastIndex, last);
311-
return new BlockStatement(statements, block.getVariableScope());
312-
}
313-
314-
if( node instanceof ExpressionStatement es ) {
315-
return new ReturnStatement(es.getExpression());
316-
}
317-
318-
if( node instanceof IfStatement ies ) {
319-
return new IfStatement(
320-
ies.getBooleanExpression(),
321-
addReturnsIfNeeded(ies.getIfBlock()),
322-
addReturnsIfNeeded(ies.getElseBlock()) );
323-
}
324-
325-
return node;
326-
}
327-
328-
@Override
329-
public void visitReturnStatement(ReturnStatement node) {
330-
var sourceType = getType(node.getExpression());
331-
if( inferredReturnType != null && !ClassHelper.isDynamicTyped(returnType) ) {
332-
if( !TypesEx.isAssignableFrom(inferredReturnType, sourceType) )
333-
addError(String.format("Return value with type %s does not match previous return type (%s)", TypesEx.getName(sourceType), TypesEx.getName(inferredReturnType)), node);
334-
}
335-
else if( TypesEx.isAssignableFrom(returnType, sourceType) ) {
336-
inferredReturnType = sourceType;
337-
}
338-
else {
339-
addError(String.format("Return value with type %s does not match the declared return type (%s)", TypesEx.getName(sourceType), TypesEx.getName(returnType)), node);
340-
}
341-
}
342-
343-
public ClassNode getInferredReturnType() {
344-
return inferredReturnType;
345-
}
346-
}
347-
348290
@Override
349291
public void visitOutput(OutputNode node) {
350292
if( !experimental ) {
@@ -1042,7 +984,7 @@ public void visitClosureExpression(ClosureExpression node) {
1042984
return;
1043985
var returnType = (ClassNode) node.getNodeMetaData(ASTNodeMarker.INFERRED_RETURN_TYPE);
1044986
if( returnType != null ) {
1045-
var visitor = new ReturnStatementVisitor();
987+
var visitor = new ReturnStatementVisitor(sourceUnit);
1046988
visitor.visit(returnType, node.getCode());
1047989

1048990
var inferredReturnType = visitor.getInferredReturnType();

0 commit comments

Comments
 (0)