Merge branch 'mm/test-grep-lint' into next

The test suite has been updated to use the 'test_grep' helper instead
of bare 'grep' for test assertions, allowing file contents to be
printed on failure for easier debugging. A new 'greplint' linter has
been introduced to detect and prevent new bare 'grep' assertions from
being added to the test suite.

* mm/test-grep-lint:
  t: add greplint to detect bare grep assertions
  t: convert grep assertions to test_grep
  t: fix Lexer line count for $() inside double-quoted strings
  t: extract chainlint's parser into shared module
  t: fix grep assertions missing file arguments
  t/README: document test_grep helper

Author: gitster

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

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,13 +40,15 @@ 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)))
 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:

t/README

@@ -1039,6 +1039,40 @@ see test-lib-functions.sh for the full list and their options.
 
    Check whether a file has the length it is expected to.
 
+ - test_grep [!] [<grep-options>] <pattern> <file>
+
+   Check whether <file> contains a line matching <pattern>, or
+   with '!' that no line matches.  Use this instead of bare
+   'grep <pattern> <file>' in test assertions.  On failure,
+   test_grep prints the contents of <file> for easier debugging,
+   whereas a bare 'grep' would fail silently.
+
+   For negation, pass '!' as the first argument:
+
+	test_grep ! "^diff --git" actual
+
+   Do not negate by writing '! test_grep', as that suppresses the
+   diagnostic output.
+
+   test_grep should only be used as a test assertion.  When grep
+   is used as a data filter (e.g. 'grep -v "^index" actual >filtered')
+   or inside a command substitution (e.g. '$(grep -c ...)'), plain
+   'grep' is the right choice because the exit code is not the
+   assertion itself.
+
+   test_grep requires <file> to exist and will BUG otherwise, so
+   use it only where the file is guaranteed to exist at that point.
+   When a file's presence is conditional (a backend-specific file,
+   or a path that only exists on some platforms, such as an NTFS
+   8.3 short name), guard the assertion on that condition (a
+   prerequisite, or a 'test -e' on the path) and use test_grep
+   inside the guard:
+
+	if test_have_prereq REFFILES
+	then
+		test_grep ! "$refname" .git/packed-refs
+	fi
+
  - test_path_is_file <path>
    test_path_is_dir <path>
    test_path_is_missing <path>