|
| 1 | +#!/usr/bin/perl |
| 2 | + |
| 3 | +# Detect bare 'grep' used as a test assertion where 'test_grep' |
| 4 | +# should be used, and '! test_grep' where 'test_grep !' should |
| 5 | +# be used. |
| 6 | +# |
| 7 | +# The shared shell parser tokenizes test bodies so that 'grep' |
| 8 | +# inside heredocs, command substitutions like $(grep ...), and |
| 9 | +# quoted strings is collapsed into a single token and never seen |
| 10 | +# by our check. A line-oriented approach would need to track |
| 11 | +# heredoc delimiters, nested $() depth, and cross-line pipe |
| 12 | +# state to avoid false positives on patterns like: |
| 13 | +# |
| 14 | +# write_script foo.sh <<-\EOF |
| 15 | +# grep pattern file # data, not an assertion |
| 16 | +# EOF |
| 17 | +# |
| 18 | +# The Lexer already handles these. |
| 19 | + |
| 20 | +use strict; |
| 21 | +use warnings; |
| 22 | +use File::Basename; |
| 23 | +do(dirname($0) . "/lib-shell-parser.pl") |
| 24 | + or die "$0: failed to load lib-shell-parser.pl: $@$!\n"; |
| 25 | + |
| 26 | +my $exit_code = 0; |
| 27 | + |
| 28 | +# GrepLintParser inherits ScriptParser's ability to find |
| 29 | +# test_expect_success/failure blocks and call check_test() |
| 30 | +# on each body. We override check_test() to walk the token |
| 31 | +# stream looking for bare grep assertions. |
| 32 | +package GrepLintParser; |
| 33 | + |
| 34 | +our @ISA = ('ScriptParser'); |
| 35 | + |
| 36 | +# After these tokens, the next token is a command word. |
| 37 | +# For example, in 'echo foo && grep bar file', the 'grep' |
| 38 | +# after '&&' is at command position and should be flagged. |
| 39 | +my %cmd_start = map { $_ => 1 } qw(&& || ; ;; do then else elif), "\n", '{', '('; |
| 40 | + |
| 41 | +# Tokens indicating grep's output is piped or redirected. |
| 42 | +my %filter_op = map { $_ => 1 } qw(| > >> <); |
| 43 | + |
| 44 | +# A token is at "command word" position if the shell would |
| 45 | +# interpret it as a program name rather than an argument. |
| 46 | +# Only 'grep' at command position is an assertion we should |
| 47 | +# flag; 'grep' as an argument ('test_must_fail grep') or |
| 48 | +# value ('for cmd in grep sed') is not. |
| 49 | +sub is_command_word { |
| 50 | + my ($tokens, $pos) = @_; |
| 51 | + return 1 if $pos == 0; |
| 52 | + for (my $j = $pos - 1; $j >= 0; $j--) { |
| 53 | + my $t = $tokens->[$j]->[0]; |
| 54 | + # After a separator or pipe, a new command starts. |
| 55 | + return 1 if $cmd_start{$t} || $t eq '|'; |
| 56 | + # After '}' or ')', what follows is a separator or |
| 57 | + # redirect on the compound command, not a new command. |
| 58 | + return 0 if $t eq '}' || $t eq ')'; |
| 59 | + # '!' is a prefix that does not consume command |
| 60 | + # position; keep scanning to find what precedes it. |
| 61 | + next if $t eq '!'; |
| 62 | + # Any other word means we are past the command word. |
| 63 | + return 0; |
| 64 | + } |
| 65 | + return 1; |
| 66 | +} |
| 67 | + |
| 68 | +# Some bare greps are intentional (e.g. file may not exist, |
| 69 | +# data filter). A '# lint-ok' annotation on the source line |
| 70 | +# suppresses the warning. |
| 71 | +sub lint_ok { |
| 72 | + my ($raw_lines, $ln) = @_; |
| 73 | + return 0 if $ln < 1 || $ln > @$raw_lines; |
| 74 | + return $raw_lines->[$ln - 1] =~ /lint-ok/; |
| 75 | +} |
| 76 | + |
| 77 | +# Grep is a filter (not an assertion) if it receives piped |
| 78 | +# input or sends its output to a pipe or redirect. Check |
| 79 | +# both directions from grep's position in the token stream. |
| 80 | +sub is_filter { |
| 81 | + my ($tokens, $pos) = @_; |
| 82 | + # Backward: is grep receiving piped input? |
| 83 | + # Newlines don't break pipes ('cmd |\n grep' is one |
| 84 | + # pipeline), so skip past them. |
| 85 | + for (my $j = $pos - 1; $j >= 0; $j--) { |
| 86 | + my $t = $tokens->[$j]->[0]; |
| 87 | + return 1 if $t eq '|'; |
| 88 | + next if $t eq "\n"; |
| 89 | + last if $cmd_start{$t} || $t eq '}' || $t eq ')'; |
| 90 | + } |
| 91 | + # Forward: is grep piping or redirecting output? |
| 92 | + for (my $j = $pos + 1; $j < @$tokens; $j++) { |
| 93 | + my $t = $tokens->[$j]->[0]; |
| 94 | + return 0 if $cmd_start{$t}; |
| 95 | + return 1 if $filter_op{$t}; |
| 96 | + } |
| 97 | + return 0; |
| 98 | +} |
| 99 | + |
| 100 | +# Map a body-relative line number to a file line number. |
| 101 | +# For double-quoted bodies, backslash-continuation lines |
| 102 | +# (\<newline>) are consumed by the Lexer without appearing |
| 103 | +# in the body text, so the inner parser sees fewer lines |
| 104 | +# than the source file has. We walk the source lines to |
| 105 | +# count continuations and adjust accordingly. |
| 106 | +sub body_to_file_line { |
| 107 | + my ($body_lineno, $body_token, $raw_lines, $lineno) = @_; |
| 108 | + my $file_lineno = $body_lineno + ($lineno || 1) - 1; |
| 109 | + # Only double-quoted bodies have line splices. |
| 110 | + return $file_lineno unless $body_token->[0] =~ /^"/; |
| 111 | + my $adj = 0; |
| 112 | + my $body_line = 0; |
| 113 | + for my $src_ln ($lineno .. ($body_token->[4] || $lineno)) { |
| 114 | + my $line = $raw_lines->[$src_ln - 1]; |
| 115 | + # Odd trailing backslashes = continuation (\<nl>). |
| 116 | + # Even = escaped backslashes (\\), not a continuation. |
| 117 | + if (defined($line) && $line =~ /(\\*)\s*$/ && |
| 118 | + length($1) % 2 == 1) { |
| 119 | + $adj++; |
| 120 | + } else { |
| 121 | + last if ++$body_line >= $body_lineno; |
| 122 | + } |
| 123 | + } |
| 124 | + return $file_lineno + $adj; |
| 125 | +} |
| 126 | + |
| 127 | +# ScriptParser calls this for each test body found in the script. |
| 128 | +sub check_test { |
| 129 | + my $self = shift @_; |
| 130 | + my $title = ScriptParser::unwrap(shift @_); |
| 131 | + my $body_token = shift @_; |
| 132 | + my $lineno = $body_token->[3]; |
| 133 | + my $body = ScriptParser::unwrap($body_token); |
| 134 | + # Handle heredoc-style test bodies: |
| 135 | + # test_expect_success 'title' - <<\EOF |
| 136 | + # grep pattern file |
| 137 | + # EOF |
| 138 | + # The '-' signals that the body follows as a heredoc. |
| 139 | + if ($body eq '-') { |
| 140 | + my $herebody = shift @_; |
| 141 | + if ($herebody) { |
| 142 | + $body = $herebody->{content}; |
| 143 | + $lineno = $herebody->{start_line}; |
| 144 | + } |
| 145 | + } |
| 146 | + return unless $body; |
| 147 | + |
| 148 | + my $raw_lines = $self->{raw_lines}; |
| 149 | + |
| 150 | + # The outer parser gives us the body as an opaque string. |
| 151 | + # Parse it to get individual tokens with command boundaries. |
| 152 | + my $parser = ShellParser->new(\$body); |
| 153 | + my @tokens = $parser->parse(); |
| 154 | + |
| 155 | + my $file = $self->{file}; |
| 156 | + |
| 157 | + for (my $i = 0; $i < @tokens; $i++) { |
| 158 | + my $text = $tokens[$i]->[0]; |
| 159 | + next unless is_command_word(\@tokens, $i); |
| 160 | + |
| 161 | + my $file_lineno = body_to_file_line( |
| 162 | + $tokens[$i]->[3] || 0, |
| 163 | + $body_token, $raw_lines, $lineno); |
| 164 | + |
| 165 | + # '!' negates the exit code without consuming command |
| 166 | + # position. '! test_grep' is an anti-pattern because |
| 167 | + # test_grep only prints diagnostics on grep failure, |
| 168 | + # and '!' inverts after that decision is already made. |
| 169 | + if ($text eq '!') { |
| 170 | + if ($i + 1 < @tokens && |
| 171 | + $tokens[$i + 1]->[0] eq 'test_grep' && |
| 172 | + !lint_ok($raw_lines, $file_lineno)) { |
| 173 | + print "$file:$file_lineno: error: ", |
| 174 | + 'use "test_grep !" instead of ', |
| 175 | + '"! test_grep"', "\n"; |
| 176 | + $exit_code = 1; |
| 177 | + } |
| 178 | + next; |
| 179 | + } |
| 180 | + |
| 181 | + # Bare grep as a command (not a filter) is a test |
| 182 | + # assertion that should use test_grep for better |
| 183 | + # failure diagnostics. |
| 184 | + if ($text eq 'grep' && |
| 185 | + !is_filter(\@tokens, $i) && |
| 186 | + !lint_ok($raw_lines, $file_lineno)) { |
| 187 | + print "$file:$file_lineno: error: ", |
| 188 | + "bare grep outside pipeline ", |
| 189 | + "(use test_grep)\n"; |
| 190 | + $exit_code = 1; |
| 191 | + } |
| 192 | + } |
| 193 | +} |
| 194 | + |
| 195 | +package main; |
| 196 | + |
| 197 | +for my $file (@ARGV) { |
| 198 | + open(my $fh, '<:unix:crlf', $file) or die "$0: $file: $!\n"; |
| 199 | + my @raw_lines = <$fh>; |
| 200 | + close $fh; |
| 201 | + my $s = join('', @raw_lines); |
| 202 | + my $parser = GrepLintParser->new(\$s); |
| 203 | + $parser->{file} = $file; |
| 204 | + $parser->{raw_lines} = \@raw_lines; |
| 205 | + $parser->parse(); |
| 206 | +} |
| 207 | +exit $exit_code; |
0 commit comments