Skip to content

Commit dd841fd

Browse files
committed
Fix type checking for output path directive
Signed-off-by: Ben Sherman <bentshermann@gmail.com>
1 parent 62d96a1 commit dd841fd

4 files changed

Lines changed: 106 additions & 32 deletions

File tree

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

Lines changed: 27 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@ public void visit() {
128128
visitProcess(processNode);
129129
for( var workflowNode : sn.getWorkflows() )
130130
visitWorkflow(workflowNode);
131+
if( sn.getOutputs() != null )
132+
visitOutputs(sn.getOutputs());
131133
}
132134
}
133135

@@ -327,18 +329,24 @@ private boolean checkPublishStatements(MethodCallExpression node, ClassNode elem
327329
closure.getParameters()[0].setType(elementType);
328330
}
329331
var code = (BlockStatement) closure.getCode();
332+
boolean checkPublish = code.getStatements().stream().anyMatch(stmt -> (
333+
asPublishStatement(stmt) != null
334+
));
330335
for( var stmt : code.getStatements() ) {
331-
if( checkPublishStatement(stmt) )
332-
continue;
333-
visit(stmt);
336+
if( checkPublish )
337+
checkPublishStatement(stmt);
338+
else
339+
visit(stmt);
334340
}
335341
return true;
336342
}
337343

338-
private boolean checkPublishStatement(Statement node) {
344+
private void checkPublishStatement(Statement node) {
339345
var publish = asPublishStatement(node);
340-
if( publish == null )
341-
return false;
346+
if( publish == null ) {
347+
addError("Statement is not a valid publish statement (hint: make sure right-hand side of `>>` expression is wrapped in parentheses)", node);
348+
return;
349+
}
342350
var source = publish.getLeftExpression();
343351
var target = publish.getRightExpression();
344352
visit(source);
@@ -349,7 +357,6 @@ private boolean checkPublishStatement(Statement node) {
349357
var targetType = getType(target);
350358
if( !TypesEx.isAssignableFrom(ClassHelper.STRING_TYPE, targetType) )
351359
addError("Publish target should be a String but was specified as a " + TypesEx.getName(targetType), target);
352-
return true;
353360
}
354361

355362
private BinaryExpression asPublishStatement(Statement node) {
@@ -405,32 +412,21 @@ else if( target instanceof TupleExpression te )
405412
}
406413

407414
private void applyTupleAssignment(TupleExpression target, ClassNode sourceType) {
415+
if( !TUPLE_TYPE.equals(sourceType) ) {
416+
addError("Tuple assignment is not compatible with type " + TypesEx.getName(sourceType), target);
417+
return;
418+
}
419+
408420
var vars = target.getExpressions();
409-
410-
if( TUPLE_TYPE.equals(sourceType) ) {
411-
var gts = sourceType.getGenericsTypes();
412-
if( gts == null )
413-
return;
414-
if( vars.size() == gts.length ) {
415-
for( int i = 0; i < vars.size(); i++ )
416-
vars.get(i).putNodeMetaData(ASTNodeMarker.INFERRED_TYPE, gts[i].getType());
417-
}
418-
else {
419-
addError("Assignment target with " + vars.size() + " components cannot be assigned to tuple with " + gts.length + " components", target);
420-
}
421+
var gts = sourceType.getGenericsTypes();
422+
if( gts == null )
423+
return;
424+
if( vars.size() == gts.length ) {
425+
for( int i = 0; i < vars.size(); i++ )
426+
vars.get(i).putNodeMetaData(ASTNodeMarker.INFERRED_TYPE, gts[i].getType());
421427
}
422-
423-
if( RECORD_TYPE.equals(sourceType) ) {
424-
var fields = sourceType.getFields();
425-
if( fields.size() == 0 )
426-
return;
427-
if( vars.size() == fields.size() ) {
428-
for( int i = 0; i < vars.size(); i++ )
429-
vars.get(i).putNodeMetaData(ASTNodeMarker.INFERRED_TYPE, fields.get(i).getType());
430-
}
431-
else {
432-
addError("Assignment target with " + vars.size() + " components cannot be assigned to record with " + fields.size() + " fields", target);
433-
}
428+
else {
429+
addError("Assignment target with " + vars.size() + " components cannot be assigned to tuple with " + gts.length + " components", target);
434430
}
435431
}
436432

src/main/java/nextflow/script/types/TypesEx.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,10 +407,15 @@ public static Class normalize(Class type) {
407407
return type;
408408
}
409409

410+
private static final List<Class> STATEFUL_TYPES = List.of(
411+
ParamsMap.class, // don't normalize since param fields are injected by params block
412+
Record.class // don't normalize since record fields are injected during type inference
413+
);
414+
410415
public static ClassNode normalize(ClassNode cn) {
411416
if( cn == null || !cn.isResolved() )
412417
return cn;
413-
if( cn.getTypeClass() == ParamsMap.class || cn.getTypeClass() == Record.class )
418+
if( STATEFUL_TYPES.contains(cn.getTypeClass()) )
414419
return cn;
415420
var result = ClassHelper.makeCached(normalize(cn.getTypeClass())).getPlainNodeReference();
416421
if( cn.getGenericsTypes() != null )

src/main/java/nextflow/script/types/shim/Float.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
""")
2626
@Ops(FloatOps.class)
2727
public interface Float {
28+
Integer intValue();
2829
}
2930

3031
interface FloatOps {

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

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,78 @@ class TypeCheckingTest extends Specification {
167167
errors.size() == 0
168168
}
169169

170+
def 'should check output path directive' () {
171+
when:
172+
def errors = getErrors(
173+
'''\
174+
nextflow.preview.types = true
175+
176+
workflow {
177+
main:
178+
ch_samples = channel.empty()
179+
180+
publish:
181+
samples = ch_samples
182+
}
183+
184+
output {
185+
samples: Channel<Sample> {
186+
path { s ->
187+
s.id >> 42
188+
s.fastq >> params.save_fastq ? 'fastq' : null
189+
}
190+
}
191+
}
192+
193+
record Sample {
194+
id: String
195+
fastq: Path
196+
}
197+
'''
198+
)
199+
then:
200+
errors.size() == 3
201+
errors[0].getStartLine() == 14
202+
errors[0].getStartColumn() == 13
203+
errors[0].getOriginalMessage() == "Publish source should be a Path or Iterable<Path> but was specified as a String"
204+
errors[1].getStartLine() == 14
205+
errors[1].getStartColumn() == 21
206+
errors[1].getOriginalMessage() == "Publish target should be a String but was specified as a Integer"
207+
errors[2].getStartLine() == 15
208+
errors[2].getStartColumn() == 13
209+
errors[2].getOriginalMessage().contains "Statement is not a valid publish statement"
210+
211+
when:
212+
errors = getErrors(
213+
'''\
214+
nextflow.preview.types = true
215+
216+
workflow {
217+
main:
218+
ch_samples = channel.empty()
219+
220+
publish:
221+
samples = ch_samples
222+
}
223+
224+
output {
225+
samples: Channel<Sample> {
226+
path { s ->
227+
s.fastq >> (params.save_fastq ? 'fastq' : null)
228+
}
229+
}
230+
}
231+
232+
record Sample {
233+
id: String
234+
fastq: Path
235+
}
236+
'''
237+
)
238+
then:
239+
errors.size() == 0
240+
}
241+
170242
def 'should check a return statement' () {
171243
when:
172244
def errors = getErrors(

0 commit comments

Comments
 (0)