Re: [PATCH 3/4] tests: drop here-doc check from internal chain-linter
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Mar 29, 2023, 03:13 UTC
- Message-ID
- <CAPig+cQexJ9s1f6tLLaq7baeVCKhazPBi_xQ6uybkQcJQ97geg@mail.gmail.com>
- In-Reply-To
- <20230329030432.GA1801645@coredump.intra.peff.net>
On Tue, Mar 28, 2023 at 11:04 PM Jeff King <peff@peff.net> wrote:
Show 23 quoted lines
> 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.
Show 18 quoted lines
> 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.