Fix error text alignment

[mahesh-panchal]

Fixes #4585

Input output

Using this toy:

workflow {
    TASK(Channel.of('a')).view()
}

process TASK {
    input:
    val letter

    script:
    assert letter.isUpperCase()
    def puzzle = null
    if ( puzzle ) {
        println "Puzzled"
    } else {
        println "LGTM"
    }
    """
    echo $letter
    """

    output:
    stdout
}

I now get a properly formatted error:

N E X T F L O W  ~  version 23.12.0-edge
Launching `../main.test.nf` [spontaneous_galileo] DSL2 - revision: 81c47780ef
[-        ] process > TASK -
ERROR ~ Error executing process > 'TASK (1)'

Caused by:
  assert letter.isUpperCase()
         |      |
         'a'    false -- Check script '/workspace/main.test.nf' at line: 10


Source block:
  assert letter.isUpperCase()
  def puzzle = null
  if ( puzzle ) {
      println "Puzzled"
  } else {
      println "LGTM"
  }
  """
  echo $letter
  """

Tip: view the complete command output by changing to the process work dir and entering the command `cat .command.out`

 -- Check '.nextflow.log' file for details

Additional notes

task.source seems to be formatted strangely such stripIndent doesn't work. As soon as it encounters indented code, all indentation is preserved. E.g.

    script:
    if (meta.id) {
        args = "-f $meta.id"
    } else {
        args = "-f $bam.baseName"
    } 

becomes:

if (meta.id) {
        args = "-f $meta.id"
    } else {
        args = "-f $bam.baseName"
    } 

[netlify]

✅ Deploy Preview for nextflow-docs-staging canceled.

Name	Link
🔨 Latest commit	fbc63c4
🔍 Latest deploy log	https://app.netlify.com/sites/nextflow-docs-staging/deploys/6633c0dec27bd90008292804

[marcodelapierre]

Hi @mahesh-panchal thank you for taking the time to open this PR.

The fix for the Error Cause looks spot on to me.

On the other hand, I would like to inspect a bit more the other fix, on the Task Source. In fact, the current approach in the PR does not seem general (see example below).
I see how task.source is formatted weirdly, perhaps investigating the root cause there will help find a general fix.

@bentsherman what do you think?

A breaking example :

• added some irregular indentations at various lines
• in the error output, the line if ( puzzle ) { has the wrong indentation

workflow {
    TASK(Channel.of('a')).view()
}

process TASK {
    input:
    val letter

    script:
    assert letter.isUpperCase()
    def puzzle = null
      if ( puzzle ) {
          println "Puzzled"
        println "Double puzzled"
    } else {
      println "LGTM"
    }
    """
    echo $letter
    """

    output:
    stdout
}

ERROR ~ Error executing process > 'TASK (1)'

Caused by:
  assert letter.isUpperCase()
         |      |
         'a'    false -- Check script 'test.nf' at line: 10


Source block:
  assert letter.isUpperCase()
  def puzzle = null
  if ( puzzle ) {
        println "Puzzled"
      println "Double puzzled"
  } else {
    println "LGTM"
  }
  """
  echo $letter
  """

[mahesh-panchal]

My problem was I couldn't find where that was coming from quickly so if someone can point me in the right direction, I can amend the PR.

[bentsherman]

The weird indentation comes from how the source code is extracted from the AST:

nextflow/modules/nextflow/src/main/groovy/nextflow/ast/NextflowDSLImpl.groovy

Lines 753 to 776 in 5c37fcc

	private void readSource( ASTNode node, StringBuilder buffer, SourceUnit unit, stripBrackets=false ) {
	final colx = node.getColumnNumber()
	final colz = node.getLastColumnNumber()
	final first = node.getLineNumber()
	final last = node.getLastLineNumber()
	for( int i=first; i<=last; i++ ) {
	def line = unit.source.getLine(i, null)
	if( i==last ) {
	line = line.substring(0,colz-1)
	if( stripBrackets ) {
	line = line.replaceFirst(/}.*$/,'')
	if( !line.trim() ) continue
	}
	}
	if( i==first ) {
	line = line.substring(colx-1)
	if( stripBrackets ) {
	line = line.replaceFirst(/^.*\{/,'').trim()
	if( !line.trim() ) continue
	}
	}
	buffer.append(line) .append('\n')
	}
	}

The main AST transform wraps the script body in a BodyDef constructor and appends the source text as an argument. Each AST node includes the line and column range for the node, so it uses that to get the source text for each statement in the script body.

The problem is that the text range doesn't include the indent before the statement, so if the statement spans multiple lines (e.g. an if-else statement), the first line is not indented.

I think the ideal solution would be to adjust readSource() to grab that first-line indent, basically set colx to 0 instead of the starting column number. But we need consider any edge cases. A line can have multiple statements separated by semi-colons... it is not "idiomatic" Nextflow, but it is valid syntax.

I will play around with this idea, and Mahesh you are more than welcome to do the same if you feel like delving into the AST 😄

[bentsherman]

ooh I can plug my developer docs: https://nextflow.io/docs/latest/developer/nextflow.ast.html

[bentsherman]

I think what we can do is check if the AST node is preceded only by whitespace on the first line and if so, include it as the indent. I will try to make this work. Also need to handle this annoying edge case with stripBrackets

[bentsherman]

Done! Works with both of your examples and also handles multi-statement lines, so the following code:

    script:
    assert letter.isUpperCase() ; def puzzle = null

Will be printed as:

    script:
    assert letter.isUpperCase()
    def puzzle = null

@marcodelapierre if you're satisfied with this solution, let's merge it

[marcodelapierre]

@bentsherman not quite there yet.

2 unit tests are failing in a quite significant way:

Condition not satisfied:

meta.getWorkflow('bravo') .source.stripIndent(true) == '''              take: foo take: bar main: print foo print bar emit: foo+bar '''.stripIndent(true)
|    |                     |      |                 |                                                                                   |
|    |                     |      |                 |                                                                                   take: foo
|    |                     |      |                 |                                                                                   take: bar
|    |                     |      |                 |                                                                                   main:
|    |                     |      |                 |                                                                                     print foo
|    |                     |      |                 |                                                                                     print bar
|    |                     |      |                 |                                                                                   emit: 
|    |                     |      |                 |                                                                                     foo+bar
|    |                     |      |                 false
|    |                     |      |                 25 differences (62% similarity)
|    |                     |      |                 (------)foo\n(------)bar\n(------~)  print foo\n  print bar\n(-----) (-~) (-)foo+bar\n
|    |                     |      |                 (take: )foo\n(take: )bar\n(main:\n)  print foo\n  print bar\n(emit:) (\n) ( )foo+bar\n
|    |                     |      foo
|    |                     |      bar
|    |                     |        print foo
|    |                     |        print bar
|    |                     |        foo+bar
|    |                                   foo
|    |                                   bar
|    |                                     print foo
|    |                                     print bar
|    |                                     foo+bar
|    WorkflowDef[workflow bravo]
<nextflow.script.ScriptMeta@40107927 clazz=class Script1 scriptPath=null definitions=[bravo:WorkflowDef[workflow bravo], alpha:WorkflowDef[workflow alpha], delta:WorkflowDef[workflow delta], empty:WorkflowDef[workflow empty]] imports=[:] module=false functionsCount=[:] memoizedMethodClosure$getModuleBundle=org.codehaus.groovy.runtime.memoize.Memoize$MemoizeFunction@51728c08>

	at nextflow.script.WorkflowDefTest.should parse workflow(WorkflowDefTest.groovy:101)

Condition not satisfied:

workflow.getSource().stripIndent(true) == '''                            take: foo main: print x emit: foo '''.stripIndent(true)
|        |           |                 |                                                                       |
|        |           |                 |                                                                       take:
|        |           |                 |                                                                         foo
|        |           |                 |                                                                       main:
|        |           |                 |                                                                         print x 
|        |           |                 |                                                                       emit: 
|        |           |                 |                                                                         foo  
|        |           |                 false
|        |           |                 28 differences (36% similarity)
|        |           |                 (------~--)foo\n(------~--)print x(-)\n(-------~--)foo(--)\n
|        |           |                 (take:\n  )foo\n(main:\n  )print x( )\n(emit: \n  )foo(  )\n
|        |           foo
|        |           print x
|        |           foo
|                        foo
|                        print x
|                        foo
WorkflowDef[workflow alpha]

	at nextflow.script.WorkflowDefTest.should capture workflow code(WorkflowDefTest.groovy:347)

[bentsherman]

The failing tests are due to changing the workflow xform to extract the source of each statement instead of the entire body, which leaves out labels and whitespace. I just adjusted the tests but I don't like warping the workflow body source this much. I think I will go back to reading the entire workflow body and strip out the brackets some other way

[mahesh-panchal]

Thanks for pushing this over the finish line.

[pditommaso]

Thanks for opening this PR!