On Tue, Mar 28, 2023 at 11:04 PM Jeff King <peff@xxxxxxxx> wrote:
> 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:
>
> @@ -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.
Yes, it would look something like that and you chose the correct spot
to detect the problem, but to get a "pretty" error message properly
positioned in the input, we need to capture the input stream position
of the here-doc tag itself in scan_heredoc_tag(). It doesn't look too
difficult, and I even started writing a bit of code to do it, but I'm
not sure how soon I can get around to finishing the implementation.
> 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.
Correct. ScriptParser scans the input files for
test_expect_{success,failure} invocations in order to extract the
individual test snippets, which then get passed to TestParser for
semantic analysis.
