[PATCH 1/3] chainlint.pl: fix line number reporting
- From
Jeff King <peff@peff.net>
- Date
- Jul 6, 2024, 06:05 UTC
- Message-ID
- <20240706060515.GA700151@coredump.intra.peff.net>
- In-Reply-To
- <20240706060143.GD698153@coredump.intra.peff.net>
The previous commit taught chainlint.pl to handle test bodies in heredocs, but there are two small bugs related to line numbers:
1. Prior to that commit, we'd leave the title and body untouched in
@_. So we could later pull the line number out of the $_[1] array
element. Now we shift off the front of the array, so we have to
remember that element to grab the line number. This is a regression
even for regular: test_expect_success 'title' '
test body
'invocations; the lines for ever test started fresh at 0.
2. For an invocation like the one above, if the test_expect_success
line is X, then "test body" would correctly start at X+1, since the
hanging newline at the start of the single-quoted test body
increments the count. But for a here-doc, there is an implicit
newline at the end of the token stream before the here-doc starts.
We have to increment "lineno" to account for this. Actually, this is not _quite_ correct, as there could be multiple
here-docs, like: test_expect_success "$(cat <<END_OF_TITLE)" - <<END_OF_TEST
this is the title
END_OF_TITLE
this is the test
END_OF_TEST in which case we'd need to skip past END_OF_TITLE. Given how
unlikely it is for anybody to do this, and since it would only
affect line numbers, it's probably not worth caring about too much.
The solution would probably be to record the starting line number
of each here-doc section in the lexer/shellparser stage.Signed-off-by: Jeff King <peff@peff.net> --- Note to the maintainer: do not worry about applying these yet! The parent message describes where they'd go in the series, but I'll send a full series once Eric and I have worked out the details. Review comments welcome, of course. :)
I actually suspect the "record the heredoc line number" thing would not be too hard. I.e., turn ShellParser's "heredoc" hash to point to hashrefs like: "{ content => ..., lineno => ... }". And that would give us a good spot to stick an "interpolate" boolean later if we want.
t/chainlint.pl | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/t/chainlint.pl b/t/chainlint.pl index eba509b8e1..c9ab79b6b0 100755 --- a/t/chainlint.pl +++ b/t/chainlint.pl @@ -620,15 +620,19 @@ sub unwrap { sub check_test { my $self = shift @_; my $title = unwrap(shift @_); - my $body = unwrap(shift @_); - $body = shift @_ if $body eq '-'; + my $body = shift @_; + my $lineno = $body->[3]; + $body = unwrap($body); + if ($body eq '-') { + $body = shift @_; + $lineno++; + } $self->{ntests}++; my $parser = TestParser->new(\$body); my @tokens = $parser->parse(); my $problems = $parser->{problems}; return unless $emit_all || @$problems; my $c = main::fd_colors(1); - my $lineno = $_[1]->[3]; my $start = 0; my $checked = ''; for (sort {$a->[1]->[2] <=> $b->[1]->[2]} @$problems) {
-- 2.45.2.1178.gaaad15bb7b