From: Jeff King <peff@peff.net>
Date: Sat, 06 Jul 2024 06:05:15 GMT
Subject: [PATCH 1/3] chainlint.pl: fix line number reporting
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