Git development
 help / color / mirror / Atom feed
From: "Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: "D. Ben Knoble" <ben.knoble@gmail.com>,
	Eric Sunshine <sunshine@sunshineco.com>,
	Michael Montalbo <mmontalbo@gmail.com>,
	Michael Montalbo <mmontalbo@gmail.com>
Subject: [PATCH v2 6/6] t: add greplint to detect bare grep assertions
Date: Sat, 13 Jun 2026 04:06:15 +0000	[thread overview]
Message-ID: <e5ecb37401502ddcdee4249a9e6ce783a4a7a487.1781323575.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.2135.v2.git.1781323575.gitgitgadget@gmail.com>

From: Michael Montalbo <mmontalbo@gmail.com>

Without a lint guard, bare grep assertions will creep back into
tests over time, defeating the previous commit's conversion.

Add greplint.pl to catch bare 'grep' used as a test assertion
(where 'test_grep' should be used) and '! test_grep' (where
'test_grep !' should be used).

greplint.pl reuses the shared shell parser from lib-shell-parser.pl
to tokenize test bodies.  The Lexer collapses heredocs, command
substitutions, and quoted strings into single tokens, so 'grep'
appearing inside these contexts is not flagged.  A flat walk over
the token stream tracks command position and pipeline state to
distinguish assertion greps from filter greps.

For double-quoted test bodies, a source-line walk counts
backslash-continuation lines that the Lexer consumes without
emitting into the body text, adjusting the reported line number
accordingly.

Add test fixtures in greplint/ (modeled on chainlint/) covering
detection of bare grep assertions, correct skipping of filters,
pipelines, redirects, command substitutions, and lint-ok annotations.

Wire into the Makefile as:
  - test-greplint: runs greplint.pl on $(T) $(THELPERS) $(TPERF)
  - check-greplint: runs greplint.pl on fixtures, diffs against expected
  - clean-greplint: removes temp dir

Add eol=lf entries in t/.gitattributes for greplint fixtures,
matching chainlint, so that check-greplint passes on Windows
where core.autocrlf would otherwise cause CRLF mismatches
between expected and actual output.

Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
---
 t/.gitattributes                              |   2 +
 t/Makefile                                    |  29 ++-
 t/greplint-cat.pl                             |  27 ++
 t/greplint.pl                                 | 241 ++++++++++++++++++
 t/greplint/bare-grep-after-and.expect         |   1 +
 t/greplint/bare-grep-after-and.test           |   4 +
 t/greplint/bare-grep-after-semicolon.expect   |   1 +
 t/greplint/bare-grep-after-semicolon.test     |   4 +
 t/greplint/bare-grep-compound-body.expect     |   3 +
 t/greplint/bare-grep-compound-body.test       |  17 ++
 t/greplint/bare-grep-count-mode.expect        |   1 +
 t/greplint/bare-grep-count-mode.test          |   3 +
 t/greplint/bare-grep-explicit-pattern.expect  |   1 +
 t/greplint/bare-grep-explicit-pattern.test    |   3 +
 t/greplint/bare-grep-flags.expect             |   1 +
 t/greplint/bare-grep-flags.test               |   3 +
 t/greplint/bare-grep-lint-ok.expect           |   0
 t/greplint/bare-grep-lint-ok.test             |   4 +
 t/greplint/bare-grep-negated.expect           |   1 +
 t/greplint/bare-grep-negated.test             |   3 +
 t/greplint/bare-grep-pattern-file.expect      |   1 +
 t/greplint/bare-grep-pattern-file.test        |   3 +
 t/greplint/bare-grep-simple.expect            |   1 +
 t/greplint/bare-grep-simple.test              |   3 +
 t/greplint/bare-grep-subshell.expect          |   1 +
 t/greplint/bare-grep-subshell.test            |   5 +
 .../dqstring-continuation-offset.expect       |   1 +
 t/greplint/dqstring-continuation-offset.test  |  11 +
 t/greplint/filter-command-substitution.expect |   0
 t/greplint/filter-command-substitution.test   |   3 +
 t/greplint/filter-pipe-input.expect           |   0
 t/greplint/filter-pipe-input.test             |   3 +
 t/greplint/filter-pipe-output.expect          |   0
 t/greplint/filter-pipe-output.test            |   3 +
 t/greplint/filter-redirect-output.expect      |   0
 t/greplint/filter-redirect-output.test        |   3 +
 t/greplint/filter-stdin-redirect.expect       |   0
 t/greplint/filter-stdin-redirect.test         |   3 +
 t/greplint/grep-as-argument.expect            |   0
 t/greplint/grep-as-argument.test              |   3 +
 t/greplint/grep-as-value.expect               |   0
 t/greplint/grep-as-value.test                 |   6 +
 t/greplint/wrong-negation.expect              |   1 +
 t/greplint/wrong-negation.test                |   3 +
 44 files changed, 398 insertions(+), 5 deletions(-)
 create mode 100644 t/greplint-cat.pl
 create mode 100644 t/greplint.pl
 create mode 100644 t/greplint/bare-grep-after-and.expect
 create mode 100644 t/greplint/bare-grep-after-and.test
 create mode 100644 t/greplint/bare-grep-after-semicolon.expect
 create mode 100644 t/greplint/bare-grep-after-semicolon.test
 create mode 100644 t/greplint/bare-grep-compound-body.expect
 create mode 100644 t/greplint/bare-grep-compound-body.test
 create mode 100644 t/greplint/bare-grep-count-mode.expect
 create mode 100644 t/greplint/bare-grep-count-mode.test
 create mode 100644 t/greplint/bare-grep-explicit-pattern.expect
 create mode 100644 t/greplint/bare-grep-explicit-pattern.test
 create mode 100644 t/greplint/bare-grep-flags.expect
 create mode 100644 t/greplint/bare-grep-flags.test
 create mode 100644 t/greplint/bare-grep-lint-ok.expect
 create mode 100644 t/greplint/bare-grep-lint-ok.test
 create mode 100644 t/greplint/bare-grep-negated.expect
 create mode 100644 t/greplint/bare-grep-negated.test
 create mode 100644 t/greplint/bare-grep-pattern-file.expect
 create mode 100644 t/greplint/bare-grep-pattern-file.test
 create mode 100644 t/greplint/bare-grep-simple.expect
 create mode 100644 t/greplint/bare-grep-simple.test
 create mode 100644 t/greplint/bare-grep-subshell.expect
 create mode 100644 t/greplint/bare-grep-subshell.test
 create mode 100644 t/greplint/dqstring-continuation-offset.expect
 create mode 100644 t/greplint/dqstring-continuation-offset.test
 create mode 100644 t/greplint/filter-command-substitution.expect
 create mode 100644 t/greplint/filter-command-substitution.test
 create mode 100644 t/greplint/filter-pipe-input.expect
 create mode 100644 t/greplint/filter-pipe-input.test
 create mode 100644 t/greplint/filter-pipe-output.expect
 create mode 100644 t/greplint/filter-pipe-output.test
 create mode 100644 t/greplint/filter-redirect-output.expect
 create mode 100644 t/greplint/filter-redirect-output.test
 create mode 100644 t/greplint/filter-stdin-redirect.expect
 create mode 100644 t/greplint/filter-stdin-redirect.test
 create mode 100644 t/greplint/grep-as-argument.expect
 create mode 100644 t/greplint/grep-as-argument.test
 create mode 100644 t/greplint/grep-as-value.expect
 create mode 100644 t/greplint/grep-as-value.test
 create mode 100644 t/greplint/wrong-negation.expect
 create mode 100644 t/greplint/wrong-negation.test

diff --git a/t/.gitattributes b/t/.gitattributes
index 7664c6e027..e867f38c71 100644
--- a/t/.gitattributes
+++ b/t/.gitattributes
@@ -1,5 +1,7 @@
 t[0-9][0-9][0-9][0-9]/* -whitespace
 /chainlint/*.expect eol=lf -whitespace
+/greplint/*.expect eol=lf -whitespace
+/greplint/*.test eol=lf -whitespace
 /t0110/url-* binary
 /t3206/* eol=lf
 /t3900/*.txt eol=lf
diff --git a/t/Makefile b/t/Makefile
index ab8a5b54aa..e3249879c4 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -27,9 +27,11 @@ TEST_LINT ?= test-lint
 ifdef TEST_OUTPUT_DIRECTORY
 TEST_RESULTS_DIRECTORY = $(TEST_OUTPUT_DIRECTORY)/test-results
 CHAINLINTTMP = $(TEST_OUTPUT_DIRECTORY)/chainlinttmp
+GREPLINTTMP = $(TEST_OUTPUT_DIRECTORY)/greplinttmp
 else
 TEST_RESULTS_DIRECTORY = test-results
 CHAINLINTTMP = chainlinttmp
+GREPLINTTMP = greplinttmp
 endif
 
 # Shell quote;
@@ -38,6 +40,7 @@ TEST_SHELL_PATH_SQ = $(subst ','\'',$(TEST_SHELL_PATH))
 PERL_PATH_SQ = $(subst ','\'',$(PERL_PATH))
 TEST_RESULTS_DIRECTORY_SQ = $(subst ','\'',$(TEST_RESULTS_DIRECTORY))
 CHAINLINTTMP_SQ = $(subst ','\'',$(CHAINLINTTMP))
+GREPLINTTMP_SQ = $(subst ','\'',$(GREPLINTTMP))
 
 T = $(sort $(wildcard t[0-9][0-9][0-9][0-9]-*.sh))
 THELPERS = $(sort $(filter-out $(T),$(wildcard *.sh)))
@@ -45,6 +48,7 @@ TLIBS = $(sort $(wildcard lib-*.sh)) annotate-tests.sh
 TPERF = $(sort $(wildcard perf/p[0-9][0-9][0-9][0-9]-*.sh))
 TINTEROP = $(sort $(wildcard interop/i[0-9][0-9][0-9][0-9]-*.sh))
 CHAINLINTTESTS = $(sort $(patsubst chainlint/%.test,%,$(wildcard chainlint/*.test)))
+GREPLINTTESTS = $(sort $(patsubst greplint/%.test,%,$(wildcard greplint/*.test)))
 CHAINLINT = '$(PERL_PATH_SQ)' chainlint.pl
 UNIT_TEST_SOURCES = $(wildcard unit-tests/t-*.c)
 UNIT_TEST_PROGRAMS = $(patsubst unit-tests/%.c,unit-tests/bin/%$(X),$(UNIT_TEST_SOURCES))
@@ -63,8 +67,8 @@ test: pre-clean check-meson $(TEST_LINT)
 	$(CHAINLINTSUPPRESS) $(MAKE) aggregate-results-and-cleanup
 
 ifneq ($(PERL_PATH),)
-test: check-chainlint
-prove: check-chainlint
+test: check-chainlint check-greplint
+prove: check-chainlint check-greplint
 endif
 
 failed:
@@ -102,7 +106,7 @@ unit-tests-test-tool:
 pre-clean:
 	$(RM) -r '$(TEST_RESULTS_DIRECTORY_SQ)'
 
-clean-except-prove-cache: clean-chainlint
+clean-except-prove-cache: clean-chainlint clean-greplint
 	$(RM) -r 'trash directory'.*
 	$(RM) -r valgrind/bin
 
@@ -120,6 +124,17 @@ check-chainlint:
 	{ $(CHAINLINT) --emit-all '$(CHAINLINTTMP_SQ)'/tests >'$(CHAINLINTTMP_SQ)'/actual || true; } && \
 	diff -u '$(CHAINLINTTMP_SQ)'/expect '$(CHAINLINTTMP_SQ)'/actual
 
+clean-greplint:
+	$(RM) -r '$(GREPLINTTMP_SQ)'
+
+check-greplint:
+	@mkdir -p '$(GREPLINTTMP_SQ)' && \
+	'$(PERL_PATH_SQ)' greplint-cat.pl '$(GREPLINTTMP_SQ)' $(GREPLINTTESTS) && \
+	{ '$(PERL_PATH_SQ)' greplint.pl \
+		$(patsubst %,greplint/%.test,$(GREPLINTTESTS)) \
+		>'$(GREPLINTTMP_SQ)'/actual 2>&1 || true; } && \
+	diff -u '$(GREPLINTTMP_SQ)'/expect '$(GREPLINTTMP_SQ)'/actual
+
 check-meson:
 	@# awk acts up when trying to match single quotes, so we use \047 instead.
 	@mkdir -p mesontmp && \
@@ -139,7 +154,7 @@ check-meson:
 test-lint: test-lint-duplicates test-lint-executable \
 	test-lint-filenames
 ifneq ($(PERL_PATH),)
-test-lint: test-lint-shell-syntax
+test-lint: test-lint-shell-syntax test-greplint
 else
 GIT_TEST_CHAIN_LINT = 0
 endif
@@ -160,6 +175,9 @@ test-lint-executable:
 test-lint-shell-syntax:
 	@'$(PERL_PATH_SQ)' check-non-portable-shell.pl $(T) $(THELPERS) $(TPERF)
 
+test-greplint:
+	@'$(PERL_PATH_SQ)' greplint.pl $(T) $(THELPERS) $(TPERF)
+
 test-lint-filenames:
 	@# We do *not* pass a glob to ls-files but use grep instead, to catch
 	@# non-ASCII characters (which are quoted within double-quotes)
@@ -185,7 +203,8 @@ perf:
 	$(MAKE) -C perf/ all
 
 .PHONY: pre-clean $(T) aggregate-results clean valgrind perf \
-	check-chainlint clean-chainlint test-chainlint $(UNIT_TESTS)
+	check-chainlint clean-chainlint test-chainlint \
+	check-greplint clean-greplint test-greplint $(UNIT_TESTS)
 
 .PHONY: libgit-sys-test libgit-rs-test
 libgit-sys-test:
diff --git a/t/greplint-cat.pl b/t/greplint-cat.pl
new file mode 100644
index 0000000000..fa9c3b8aab
--- /dev/null
+++ b/t/greplint-cat.pl
@@ -0,0 +1,27 @@
+#!/usr/bin/env perl
+
+use strict;
+use warnings;
+
+# Assemble expected output for check-greplint target.
+# Usage: greplint-cat.pl <outdir> <test-name> ...
+#
+# For each <test-name>, reads greplint/<test-name>.expect and
+# prepends "greplint/<test-name>.test:" to every non-empty line,
+# matching the output format of greplint.pl.  Writes combined
+# expected output to <outdir>/expect.
+
+my $outdir = shift;
+open(my $expect, '>', "$outdir/expect")
+	or die "unable to open $outdir/expect: $!";
+
+for my $name (@ARGV) {
+	open(my $fh, '<', "greplint/$name.expect")
+		or die "unable to open greplint/$name.expect: $!";
+	while (<$fh>) {
+		print $expect "greplint/$name.test:$_";
+	}
+	close $fh;
+}
+
+close $expect;
diff --git a/t/greplint.pl b/t/greplint.pl
new file mode 100644
index 0000000000..553bdbdf9a
--- /dev/null
+++ b/t/greplint.pl
@@ -0,0 +1,241 @@
+#!/usr/bin/env perl
+
+# Detect bare 'grep' used as a test assertion where 'test_grep'
+# should be used, and '! test_grep' where 'test_grep !' should
+# be used.
+#
+# The shared shell parser tokenizes test bodies so that 'grep'
+# inside heredocs, command substitutions like $(grep ...), and
+# quoted strings is collapsed into a single token and never seen
+# by our check.  A line-oriented approach would need to track
+# heredoc delimiters, nested $() depth, and cross-line pipe
+# state to avoid false positives on patterns like:
+#
+#   write_script foo.sh <<-\EOF
+#   grep pattern file    # data, not an assertion
+#   EOF
+#
+# The Lexer already handles these.
+
+use warnings;
+use strict;
+use File::Basename;
+do(dirname($0) . "/lib-shell-parser.pl")
+	or die "$0: failed to load lib-shell-parser.pl: $@$!\n";
+
+my $exit_code = 0;
+
+# GrepLintParser inherits ScriptParser's ability to find
+# test_expect_success/failure blocks and call check_test()
+# on each body.  We override check_test() to walk the token
+# stream looking for bare grep assertions.
+package GrepLintParser;
+
+our @ISA = ('ScriptParser');
+
+# After these tokens, the next token is a command word.
+# For example, in 'echo foo && grep bar file', the 'grep'
+# after '&&' is at command position and should be flagged.
+my %cmd_start = map { $_ => 1 } qw(&& || ; ;; do then else elif), "\n", '{', '(';
+
+# Tokens indicating grep's output is piped or redirected.
+my %filter_op = map { $_ => 1 } qw(| > >> <);
+
+# A token is at "command word" position if the shell would
+# interpret it as a program name rather than an argument.
+# Only 'grep' at command position is an assertion we should
+# flag; 'grep' as an argument ('test_must_fail grep') or
+# value ('for cmd in grep sed') is not.
+sub is_command_word {
+	my ($tokens, $pos) = @_;
+	return 1 if $pos == 0;
+	for (my $j = $pos - 1; $j >= 0; $j--) {
+		my $t = $tokens->[$j]->[0];
+		# After a separator or pipe, a new command starts.
+		return 1 if $cmd_start{$t} || $t eq '|';
+		# After '}' or ')', what follows is a separator or
+		# redirect on the compound command, not a new command.
+		return 0 if $t eq '}' || $t eq ')';
+		# '!' is a prefix that does not consume command
+		# position; keep scanning to find what precedes it.
+		next if $t eq '!';
+		# Any other word means we are past the command word.
+		return 0;
+	}
+	return 1;
+}
+
+# Some bare greps are intentional (e.g. file may not exist,
+# data filter).  A '# lint-ok' annotation on the source line
+# suppresses the warning.
+sub lint_ok {
+	my ($raw_lines, $ln) = @_;
+	if ($ln < 1 || $ln > @$raw_lines) {
+		warn "lint_ok: line number $ln out of range (1.." .
+		    scalar(@$raw_lines) . ")\n";
+		return 0;
+	}
+	return $raw_lines->[$ln - 1] =~ /lint-ok/;
+}
+
+# Grep is a filter (not an assertion) if it receives piped
+# input or sends its output to a pipe or redirect.  Check
+# both directions from grep's position in the token stream.
+sub is_filter {
+	my ($tokens, $pos) = @_;
+	# Backward: is grep receiving piped input?
+	# Newlines don't break pipes ('cmd |\n grep' is one
+	# pipeline), so skip past them.
+	for (my $j = $pos - 1; $j >= 0; $j--) {
+		my $t = $tokens->[$j]->[0];
+		return 1 if $t eq '|';
+		next if $t eq "\n";
+		last if $cmd_start{$t} || $t eq '}' || $t eq ')';
+	}
+	# Forward: is grep piping or redirecting output?
+	# Unlike the backward scan, we do not skip newlines here:
+	# a bare newline is a command boundary, and redirects or
+	# pipes must appear on the same line as grep (or after a
+	# line continuation, which the Lexer consumes).
+	for (my $j = $pos + 1; $j < @$tokens; $j++) {
+		my $t = $tokens->[$j]->[0];
+		return 0 if $cmd_start{$t};
+		return 1 if $filter_op{$t};
+	}
+	return 0;
+}
+
+# Map a body-relative line number to a file line number.
+# For double-quoted bodies, backslash-continuation lines
+# (\<newline>) are consumed by the Lexer without appearing
+# in the body text, so the inner parser sees fewer lines
+# than the source file has.  We walk the source lines to
+# count continuations and adjust accordingly.
+sub body_to_file_line {
+	my ($body_lineno, $body_token, $raw_lines, $body_start) = @_;
+	my $body_text = $body_token->[0];
+	my $body_end_line = $body_token->[4];
+	unless ($body_start && $body_start >= 1) {
+		warn "body_start is not a positive integer\n";
+		return $body_lineno;
+	}
+	my $file_lineno = $body_lineno + $body_start - 1;
+	# Only double-quoted bodies have line splices.
+	return $file_lineno unless $body_text =~ /^"/;
+	my $adj = 0;
+	my $lines_seen = 0;
+	unless ($body_end_line && $body_end_line >= $body_start) {
+		warn "body_end_line is not set for double-quoted body\n";
+		return $file_lineno;
+	}
+	my $end = $body_end_line;
+	if ($end > @$raw_lines) {
+		warn "body_end_line ($end) exceeds file length (" .
+		    scalar(@$raw_lines) . ")\n";
+		return $file_lineno;
+	}
+	my $src_ln = $body_start;
+	while ($src_ln <= $end && $lines_seen < $body_lineno) {
+		my $line = $raw_lines->[$src_ln - 1];
+		# Odd trailing backslashes = continuation (\<nl>).
+		# Even = escaped backslashes (\\), not a continuation.
+		if ($line =~ /(\\*)$/ && length($1) % 2 == 1) {
+			$adj++;
+		} else {
+			$lines_seen++;
+		}
+		$src_ln++;
+	}
+	if ($lines_seen < $body_lineno) {
+		warn "body_lineno ($body_lineno) not found within body range " .
+		    "($body_start..$end)\n";
+	}
+	return $file_lineno + $adj;
+}
+
+# ScriptParser calls this for each test body found in the script.
+sub check_test {
+	my $self = shift @_;
+	my $title = ScriptParser::unwrap(shift @_);
+	my $body_token = shift @_;
+	my $body_start = $body_token->[3];
+	my $body = ScriptParser::unwrap($body_token);
+	# Handle heredoc-style test bodies:
+	#   test_expect_success 'title' - <<\EOF
+	#   grep pattern file
+	#   EOF
+	# The '-' signals that the body follows as a heredoc.
+	if ($body eq '-') {
+		my $herebody = shift @_;
+		if ($herebody) {
+			$body = $herebody->{content};
+			$body_start = $herebody->{start_line};
+		}
+	}
+	return unless $body;
+
+	my $raw_lines = $self->{raw_lines};
+
+	# The outer parser gives us the body as an opaque string.
+	# Parse it to get individual tokens with command boundaries.
+	my $parser = ShellParser->new(\$body);
+	my @tokens = $parser->parse();
+
+	my $file = $self->{file};
+
+	for (my $i = 0; $i < @tokens; $i++) {
+		my $text = $tokens[$i]->[0];
+		next unless is_command_word(\@tokens, $i);
+
+		my $token_lineno = $tokens[$i]->[3];
+		unless (defined($token_lineno) && $token_lineno >= 1) {
+			warn "token has no line number\n";
+			next;
+		}
+		my $file_lineno = body_to_file_line(
+			$token_lineno,
+			$body_token, $raw_lines, $body_start);
+
+		# '!' negates the exit code without consuming command
+		# position.  '! test_grep' is an anti-pattern because
+		# test_grep only prints diagnostics on grep failure,
+		# and '!' inverts after that decision is already made.
+		if ($text eq '!') {
+			if ($i + 1 < @tokens &&
+			    $tokens[$i + 1]->[0] eq 'test_grep' &&
+			    !lint_ok($raw_lines, $file_lineno)) {
+				print "$file:$file_lineno: error: ",
+				    'use "test_grep !" instead of ',
+				    '"! test_grep"', "\n";
+				$exit_code = 1;
+			}
+			next;
+		}
+
+		# Bare grep as a command (not a filter) is a test
+		# assertion that should use test_grep for better
+		# failure diagnostics.
+		if ($text eq 'grep' &&
+		    !is_filter(\@tokens, $i) &&
+		    !lint_ok($raw_lines, $file_lineno)) {
+			print "$file:$file_lineno: error: ",
+			    "bare grep outside pipeline ",
+			    "(use test_grep)\n";
+			$exit_code = 1;
+		}
+	}
+}
+
+package main;
+
+for my $file (@ARGV) {
+	open(my $fh, '<:unix:crlf', $file) or die "$0: $file: $!\n";
+	my @raw_lines = <$fh>;
+	close $fh;
+	my $s = join('', @raw_lines);
+	my $parser = GrepLintParser->new(\$s);
+	$parser->{file} = $file;
+	$parser->{raw_lines} = \@raw_lines;
+	$parser->parse();
+}
+exit $exit_code;
diff --git a/t/greplint/bare-grep-after-and.expect b/t/greplint/bare-grep-after-and.expect
new file mode 100644
index 0000000000..7da1b21aa8
--- /dev/null
+++ b/t/greplint/bare-grep-after-and.expect
@@ -0,0 +1 @@
+3: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-after-and.test b/t/greplint/bare-grep-after-and.test
new file mode 100644
index 0000000000..c5b26e4572
--- /dev/null
+++ b/t/greplint/bare-grep-after-and.test
@@ -0,0 +1,4 @@
+test_expect_success 'grep after && is flagged' '
+	cmd &&
+	grep pattern file
+'
diff --git a/t/greplint/bare-grep-after-semicolon.expect b/t/greplint/bare-grep-after-semicolon.expect
new file mode 100644
index 0000000000..7da1b21aa8
--- /dev/null
+++ b/t/greplint/bare-grep-after-semicolon.expect
@@ -0,0 +1 @@
+3: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-after-semicolon.test b/t/greplint/bare-grep-after-semicolon.test
new file mode 100644
index 0000000000..c1e468ddf8
--- /dev/null
+++ b/t/greplint/bare-grep-after-semicolon.test
@@ -0,0 +1,4 @@
+test_expect_success 'grep after semicolon is flagged' '
+	echo hello;
+	grep pattern file
+'
diff --git a/t/greplint/bare-grep-compound-body.expect b/t/greplint/bare-grep-compound-body.expect
new file mode 100644
index 0000000000..eb6210f0c9
--- /dev/null
+++ b/t/greplint/bare-grep-compound-body.expect
@@ -0,0 +1,3 @@
+4: error: bare grep outside pipeline (use test_grep)
+8: error: bare grep outside pipeline (use test_grep)
+15: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-compound-body.test b/t/greplint/bare-grep-compound-body.test
new file mode 100644
index 0000000000..72472edcf6
--- /dev/null
+++ b/t/greplint/bare-grep-compound-body.test
@@ -0,0 +1,17 @@
+test_expect_success 'grep after then/do/else is flagged' '
+	if true
+	then
+		grep pattern file
+	fi &&
+	while true
+	do
+		grep pattern file &&
+		break
+	done &&
+	if true
+	then
+		echo yes
+	else
+		grep pattern file
+	fi
+'
diff --git a/t/greplint/bare-grep-count-mode.expect b/t/greplint/bare-grep-count-mode.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-count-mode.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-count-mode.test b/t/greplint/bare-grep-count-mode.test
new file mode 100644
index 0000000000..b9c0f18783
--- /dev/null
+++ b/t/greplint/bare-grep-count-mode.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep -c is flagged (not special-cased)' '
+	grep -c pattern file
+'
diff --git a/t/greplint/bare-grep-explicit-pattern.expect b/t/greplint/bare-grep-explicit-pattern.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-explicit-pattern.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-explicit-pattern.test b/t/greplint/bare-grep-explicit-pattern.test
new file mode 100644
index 0000000000..69d8651019
--- /dev/null
+++ b/t/greplint/bare-grep-explicit-pattern.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep -e is flagged' '
+	grep -e pattern file
+'
diff --git a/t/greplint/bare-grep-flags.expect b/t/greplint/bare-grep-flags.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-flags.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-flags.test b/t/greplint/bare-grep-flags.test
new file mode 100644
index 0000000000..9ca0e10233
--- /dev/null
+++ b/t/greplint/bare-grep-flags.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep -E is flagged' '
+	grep -E "pat+ern" file
+'
diff --git a/t/greplint/bare-grep-lint-ok.expect b/t/greplint/bare-grep-lint-ok.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/bare-grep-lint-ok.test b/t/greplint/bare-grep-lint-ok.test
new file mode 100644
index 0000000000..335b902a42
--- /dev/null
+++ b/t/greplint/bare-grep-lint-ok.test
@@ -0,0 +1,4 @@
+test_expect_success 'grep with lint-ok annotation is not flagged' '
+	grep pattern file && # lint-ok
+	echo done
+'
diff --git a/t/greplint/bare-grep-negated.expect b/t/greplint/bare-grep-negated.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-negated.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-negated.test b/t/greplint/bare-grep-negated.test
new file mode 100644
index 0000000000..ef789bc8b7
--- /dev/null
+++ b/t/greplint/bare-grep-negated.test
@@ -0,0 +1,3 @@
+test_expect_success 'negated grep is flagged' '
+	! grep pattern file
+'
diff --git a/t/greplint/bare-grep-pattern-file.expect b/t/greplint/bare-grep-pattern-file.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-pattern-file.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-pattern-file.test b/t/greplint/bare-grep-pattern-file.test
new file mode 100644
index 0000000000..d70035e449
--- /dev/null
+++ b/t/greplint/bare-grep-pattern-file.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep -f is flagged' '
+	grep -f patterns.txt file
+'
diff --git a/t/greplint/bare-grep-simple.expect b/t/greplint/bare-grep-simple.expect
new file mode 100644
index 0000000000..8922d35b42
--- /dev/null
+++ b/t/greplint/bare-grep-simple.expect
@@ -0,0 +1 @@
+2: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-simple.test b/t/greplint/bare-grep-simple.test
new file mode 100644
index 0000000000..3a8d5f9bbd
--- /dev/null
+++ b/t/greplint/bare-grep-simple.test
@@ -0,0 +1,3 @@
+test_expect_success 'bare grep is flagged' '
+	grep pattern file
+'
diff --git a/t/greplint/bare-grep-subshell.expect b/t/greplint/bare-grep-subshell.expect
new file mode 100644
index 0000000000..7da1b21aa8
--- /dev/null
+++ b/t/greplint/bare-grep-subshell.expect
@@ -0,0 +1 @@
+3: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/bare-grep-subshell.test b/t/greplint/bare-grep-subshell.test
new file mode 100644
index 0000000000..1fab2d3363
--- /dev/null
+++ b/t/greplint/bare-grep-subshell.test
@@ -0,0 +1,5 @@
+test_expect_success 'grep in subshell is flagged' '
+	(
+		grep pattern file
+	)
+'
diff --git a/t/greplint/dqstring-continuation-offset.expect b/t/greplint/dqstring-continuation-offset.expect
new file mode 100644
index 0000000000..8185bfdca4
--- /dev/null
+++ b/t/greplint/dqstring-continuation-offset.expect
@@ -0,0 +1 @@
+10: error: bare grep outside pipeline (use test_grep)
diff --git a/t/greplint/dqstring-continuation-offset.test b/t/greplint/dqstring-continuation-offset.test
new file mode 100644
index 0000000000..b332bd8f3b
--- /dev/null
+++ b/t/greplint/dqstring-continuation-offset.test
@@ -0,0 +1,11 @@
+# Double-quoted test bodies with backslash-continuation lines:
+# the splice adjustment in check_test compensates for \<newline>
+# lines that the lexer consumes without emitting into the body
+# text, so the reported line number matches the source.
+test_expect_success 'dqstring continuation offset' "
+	x=\$(echo \
+		hello) &&
+	y=\$(echo \
+		world) &&
+	grep pattern file
+"
diff --git a/t/greplint/filter-command-substitution.expect b/t/greplint/filter-command-substitution.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/filter-command-substitution.test b/t/greplint/filter-command-substitution.test
new file mode 100644
index 0000000000..bd9a100a5d
--- /dev/null
+++ b/t/greplint/filter-command-substitution.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep in command substitution is not flagged' '
+	x=$(grep pattern file)
+'
diff --git a/t/greplint/filter-pipe-input.expect b/t/greplint/filter-pipe-input.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/filter-pipe-input.test b/t/greplint/filter-pipe-input.test
new file mode 100644
index 0000000000..cb5c0d9e23
--- /dev/null
+++ b/t/greplint/filter-pipe-input.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep receiving pipe input is not flagged' '
+	cmd | grep pattern
+'
diff --git a/t/greplint/filter-pipe-output.expect b/t/greplint/filter-pipe-output.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/filter-pipe-output.test b/t/greplint/filter-pipe-output.test
new file mode 100644
index 0000000000..01340086d7
--- /dev/null
+++ b/t/greplint/filter-pipe-output.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep piping to another command is not flagged' '
+	grep pattern file | wc -l
+'
diff --git a/t/greplint/filter-redirect-output.expect b/t/greplint/filter-redirect-output.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/filter-redirect-output.test b/t/greplint/filter-redirect-output.test
new file mode 100644
index 0000000000..ee70ba7cde
--- /dev/null
+++ b/t/greplint/filter-redirect-output.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep with output redirect is not flagged' '
+	grep pattern file >output
+'
diff --git a/t/greplint/filter-stdin-redirect.expect b/t/greplint/filter-stdin-redirect.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/filter-stdin-redirect.test b/t/greplint/filter-stdin-redirect.test
new file mode 100644
index 0000000000..292db7766e
--- /dev/null
+++ b/t/greplint/filter-stdin-redirect.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep reading from stdin redirect is not flagged' '
+	grep pattern <input
+'
diff --git a/t/greplint/grep-as-argument.expect b/t/greplint/grep-as-argument.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/grep-as-argument.test b/t/greplint/grep-as-argument.test
new file mode 100644
index 0000000000..7157228b60
--- /dev/null
+++ b/t/greplint/grep-as-argument.test
@@ -0,0 +1,3 @@
+test_expect_success 'grep as argument to another command is not flagged' '
+	test_must_fail grep pattern file
+'
diff --git a/t/greplint/grep-as-value.expect b/t/greplint/grep-as-value.expect
new file mode 100644
index 0000000000..e69de29bb2
diff --git a/t/greplint/grep-as-value.test b/t/greplint/grep-as-value.test
new file mode 100644
index 0000000000..92926f6378
--- /dev/null
+++ b/t/greplint/grep-as-value.test
@@ -0,0 +1,6 @@
+test_expect_success 'grep as value in for-loop is not flagged' '
+	for cmd in grep sed awk
+	do
+		echo $cmd
+	done
+'
diff --git a/t/greplint/wrong-negation.expect b/t/greplint/wrong-negation.expect
new file mode 100644
index 0000000000..7dc52f65a8
--- /dev/null
+++ b/t/greplint/wrong-negation.expect
@@ -0,0 +1 @@
+2: error: use "test_grep !" instead of "! test_grep"
diff --git a/t/greplint/wrong-negation.test b/t/greplint/wrong-negation.test
new file mode 100644
index 0000000000..542fbd9b28
--- /dev/null
+++ b/t/greplint/wrong-negation.test
@@ -0,0 +1,3 @@
+test_expect_success 'wrong negation of test_grep is flagged' '
+	! test_grep pattern file
+'
-- 
gitgitgadget

      parent reply	other threads:[~2026-06-13  4:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-04  7:45 [PATCH 0/6] t: add lint-style.pl and convert grep to test_grep Michael Montalbo via GitGitGadget
2026-06-04  7:45 ` [PATCH 1/6] t/README: document test_grep helper Michael Montalbo via GitGitGadget
2026-06-04  7:45 ` [PATCH 2/6] t: extract chainlint's parser into shared module Michael Montalbo via GitGitGadget
2026-06-04  7:45 ` [PATCH 3/6] t: fix Lexer line count for $() inside double-quoted strings Michael Montalbo via GitGitGadget
2026-06-04  7:45 ` [PATCH 4/6] t: add lint-style.pl with test_grep negation rule Michael Montalbo via GitGitGadget
2026-06-04 18:34   ` D. Ben Knoble
2026-06-04 19:36     ` Michael Montalbo
2026-06-04  7:45 ` [PATCH 5/6] t: fix grep assertions missing file arguments Michael Montalbo via GitGitGadget
2026-06-04  7:45 ` [PATCH 6/6] t: lint and convert grep assertions to test_grep Michael Montalbo via GitGitGadget
2026-06-08 21:36 ` [PATCH 0/6] t: add lint-style.pl and convert grep " Junio C Hamano
2026-06-13  4:06 ` [PATCH v2 0/6] t: add greplint.pl " Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` [PATCH v2 1/6] t/README: document test_grep helper Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` [PATCH v2 2/6] t: fix grep assertions missing file arguments Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` [PATCH v2 3/6] t: extract chainlint's parser into shared module Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` [PATCH v2 4/6] t: fix Lexer line count for $() inside double-quoted strings Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` [PATCH v2 5/6] t: convert grep assertions to test_grep Michael Montalbo via GitGitGadget
2026-06-13  4:06   ` Michael Montalbo via GitGitGadget [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e5ecb37401502ddcdee4249a9e6ce783a4a7a487.1781323575.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=ben.knoble@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mmontalbo@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox