Re: [PATCH 3/4] tests: drop here-doc check from internal chain-linter
On Tue, Mar 28, 2023 at 10:37:02PM -0400, Jeff King wrote:
> I just think chainlint.pl is doing a good enough job of catching it that
> we can rely on it. I'll be curious if Eric has input there on whether it
> can do even better, which would remove all of the caveats from the
> commit message.
So I _think_ it's something like this:
diff --git a/t/chainlint.pl b/t/chainlint.pl
index e966412999a..6b8c1de5208 100755
--- a/t/chainlint.pl
+++ b/t/chainlint.pl
@@ -171,6 +171,9 @@ sub swallow_heredocs {
my $start = pos($$b);
my $indent = $tag =~ s/^\t// ? '\\s*' : '';
$$b =~ /(?:\G|\n)$indent\Q$tag\E(?:\n|\z)/gc;
+ if (pos($$b) == $start) {
+ die "oops, we did not find the end of the heredoc";
+ }
my $body = substr($$b, $start, pos($$b) - $start);
$self->{lineno} += () = $body =~ /\n/sg;
}
But I wasn't sure how to surface a clean error from here, since we're in
the Lexer. Maybe we just accumulate a "problems" array here, and then
roll those up via the TestParser? I'm not very familiar with the
arrangement of that part of the script.
And I say "think" because the thing I was worried about is that we'd do
this lexing at too high a level, and accidentally walk past the end of
the test. Which would mean getting fooled by;
test_expect_success 'this one is broken' '
cat >foo <<\EOF
oops, we are missing our here-doc end
'
test_expect_success 'this one is ok' '
cat >foo <<\EOF
this one is OK, but we would not want to confuse
its closing tag for the missing one
EOF
'
But it looks like Lexer::swallow_heredocs gets to see the individual
test snippets.
-Peff
Jeff King <peff@peff.net>