Address review comments

godaddy · Nov 4, 2021 · 93ef4d4 · 93ef4d4
1 parent 3297420
commit 93ef4d4
Show file tree

Hide file tree

Showing 4 changed files with 17 additions and 40 deletions.
diff --git a/README.md b/README.md
@@ -59,7 +59,7 @@ Options:
                                   [default: False]
   --scan-filenames / --no-scan-filenames            
                                   Enable scanning of file names.
-                                  [default: False]
+                                  [default: True]
 
   -i, --include-paths FILENAME    [DEPRECATED] Use `--include-path-patterns`.
                                   File with regular expressions (one per

diff --git a/tartufo/cli.py b/tartufo/cli.py
@@ -85,8 +85,8 @@ def get_command(self, ctx: click.Context, cmd_name: str) -> Optional[click.Comma
     "--scan-filenames/--no-scan-filenames",
     is_flag=True,
     default=True,
-    show_default="--scan-filenames",
-    help="Enable filename checks.",
+    show_default=True,
+    help="Check the names of files being scanned as well as their contents.",
 )
 @click.option(
     "-i",

diff --git a/tartufo/scanner.py b/tartufo/scanner.py
@@ -520,7 +520,7 @@ def __init__(self, global_options: types.GlobalOptions, repo_path: str) -> None:
         self._repo = self.load_repo(self.repo_path)
 
     def _iter_diff_index(
-        self, diff: pygit2.Diff, scan_filename: bool
+        self, diff: pygit2.Diff
     ) -> Generator[Tuple[str, str], None, None]:
         """Iterate over a "diff index", yielding the individual file changes.
 
@@ -546,12 +546,12 @@ def _iter_diff_index(
                 self.logger.debug("Skipping as the file is deleted")
                 continue
             printable_diff: str = patch.text
-            if not scan_filename:
+            if not self.global_options.scan_filenames:
+                # The `printable_diff` contains diff header,
+                # so we need to strip that before analyzing it
                 header_length = GitScanner.header_length(printable_diff)
                 printable_diff = printable_diff[header_length:]
             if self.should_scan(file_path):
-                # The `printable_diff` contains the variable length diff header,
-                # so we need to strip that before analyzing it
                 yield printable_diff, file_path
 
     @staticmethod
@@ -707,9 +707,7 @@ def chunks(self) -> Generator[types.Chunk, None, None]:
                     continue
                 already_searched.add(diff_hash)
                 diff.find_similar()
-                for blob, file_path in self._iter_diff_index(
-                    diff, self.global_options.scan_filenames
-                ):
+                for blob, file_path in self._iter_diff_index(diff):
                     yield types.Chunk(
                         blob,
                         file_path,
@@ -720,9 +718,7 @@ def chunks(self) -> Generator[types.Chunk, None, None]:
             if curr_commit:
                 tree: pygit2.Tree = self._repo.revparse_single(curr_commit.hex).tree
                 tree_diff: pygit2.Diff = tree.diff_to_tree(swap=True)
-                iter_diff = self._iter_diff_index(
-                    tree_diff, self.global_options.scan_filenames
-                )
+                iter_diff = self._iter_diff_index(tree_diff)
                 for blob, file_path in iter_diff:
                     yield types.Chunk(
                         blob,
@@ -756,9 +752,7 @@ def chunks(self):
         :rtype: Generator[Chunk, None, None]
         """
         diff_index = self._repo.diff("HEAD")
-        for blob, file_path in self._iter_diff_index(
-            diff_index, self.global_options.scan_filenames
-        ):
+        for blob, file_path in self._iter_diff_index(diff_index):
             yield types.Chunk(blob, file_path, {})
 
 
@@ -803,6 +797,8 @@ def _iter_folder(self) -> Generator[Tuple[str, str], None, None]:
 
                 try:
                     blob = data.decode("utf-8")
+                    if self.global_options.scan_filenames:
+                        blob = str(relative_path) + "\n" + blob
                 except UnicodeDecodeError:
                     # binary file, continue
                     continue

diff --git a/tests/test_git_repo_scanner.py b/tests/test_git_repo_scanner.py
@@ -251,15 +251,12 @@ def test_all_commits_are_scanned_for_files(
             (
                 mock.call(
                     mock_repo.return_value.diff(mock_commit_3, mock_commit_2),
-                    self.global_options.scan_filenames,
                 ),
                 mock.call(
                     mock_repo.return_value.diff(mock_commit_2, mock_commit_1),
-                    self.global_options.scan_filenames,
                 ),
                 mock.call(
                     mock_repo.return_value.revparse_single().tree.diff_to_tree(),
-                    self.global_options.scan_filenames,
                 ),
             )
         )
@@ -310,11 +307,7 @@ def test_binary_files_are_skipped(self):
         test_scanner = scanner.GitRepoScanner(
             self.global_options, self.git_options, "."
         )
-        diffs = list(
-            test_scanner._iter_diff_index(
-                [mock_diff], self.global_options.scan_filenames
-            )
-        )
+        diffs = list(test_scanner._iter_diff_index([mock_diff]))
         self.assertEqual(diffs, [])
 
     @mock.patch("pygit2.Repository", new=mock.MagicMock())
@@ -326,11 +319,7 @@ def test_excluded_files_are_not_scanned(self, mock_should: mock.MagicMock):
         test_scanner = scanner.GitRepoScanner(
             self.global_options, self.git_options, "."
         )
-        diffs = list(
-            test_scanner._iter_diff_index(
-                [mock_diff], self.global_options.scan_filenames
-            )
-        )
+        diffs = list(test_scanner._iter_diff_index([mock_diff]))
         self.assertEqual(diffs, [])
         mock_should.assert_called_once()
 
@@ -363,11 +352,7 @@ def test_all_files_are_yielded(self, mock_should: mock.MagicMock):
         test_scanner = scanner.GitRepoScanner(
             self.global_options, self.git_options, "."
         )
-        diffs = list(
-            test_scanner._iter_diff_index(
-                [mock_diff_1, mock_diff_2], self.global_options.scan_filenames
-            )
-        )
+        diffs = list(test_scanner._iter_diff_index([mock_diff_1, mock_diff_2]))
         self.assertEqual(
             diffs,
             [
@@ -414,9 +399,7 @@ def test_scan_filename_disabled(self, mock_header_length):
             self.global_options, self.git_options, "."
         )
 
-        for _ in test_scanner._iter_diff_index(
-            [mock_diff], self.global_options.scan_filenames
-        ):
+        for _ in test_scanner._iter_diff_index([mock_diff]):
             pass
 
         mock_header_length.assert_called_once_with(mock_diff.text)
@@ -431,9 +414,7 @@ def test_scan_filename_enabled(self, mock_header_length):
             self.global_options, self.git_options, "."
         )
 
-        for _ in test_scanner._iter_diff_index(
-            [mock_diff], self.global_options.scan_filenames
-        ):
+        for _ in test_scanner._iter_diff_index([mock_diff]):
             pass
 
         mock_header_length.assert_not_called()