Reland "[presubmit checks] Check if files are written to a dep dir"

This is a reland of commit 561772c448

Original change's description:
> [presubmit checks] Check if files are written to a dep dir
>
> No files should be written to a directory that's used by CIPD or GCS, as
> defined in DEPS. Git already doesn't allow files to be written to a
> directory that's a gitlink.
>
> R=jojwang@google.com
>
> Bug: 343199633
> Change-Id: I8d3414eac728580eaf9ac7e337bb22bca3989e4e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5633187
> Reviewed-by: Gavin Mak <gavinmak@google.com>
> Commit-Queue: Josip Sokcevic <sokcevic@chromium.org>

Bug: 343199633
Change-Id: Ifc3f6c4df328cdd215ceb7d0333f68223e9a1ccb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/5643922
Reviewed-by: Gavin Mak <gavinmak@google.com>
Commit-Queue: Gavin Mak <gavinmak@google.com>
Auto-Submit: Josip Sokcevic <sokcevic@chromium.org>

presubmit_canned_checks.py

@@ -1766,6 +1766,9 @@ def PanProjectChecks(input_api,
                 input_api, output_api))
 
     if global_checks:
+        results.extend(
+            input_api.canned_checks.CheckNoNewGitFilesAddedInDependencies(
+                input_api, output_api))
         if input_api.change.scm == 'git':
             snapshot("checking for commit objects in tree")
             results.extend(
@@ -2136,6 +2139,46 @@ def CheckForRecursedeps(input_api, output_api):
     return errors
 
 
+def _readDeps(input_api):
+    """Read DEPS file from the checkout disk. Extracted for testability."""
+    deps_file = input_api.os_path.join(input_api.PresubmitLocalPath(), 'DEPS')
+    with open(deps_file) as f:
+        return f.read()
+
+
+def CheckNoNewGitFilesAddedInDependencies(input_api, output_api):
+    """Check if there are Git files in any DEPS dependencies. Error is returned
+    if there are."""
+    try:
+        deps = _ParseDeps(_readDeps(input_api))
+    except FileNotFoundError:
+        # If there's no DEPS file, there is nothing to check.
+        return []
+
+    dependency_paths = set()
+    for path, dep in deps.get('deps', {}).items():
+        if 'condition' in dep and 'non_git_source' in dep['condition']:
+            # TODO(crbug.com/40738689): Remove src/ prefix
+            dependency_paths.add(path[4:])  # 4 == len('src/')
+
+    errors = []
+    for file in input_api.AffectedFiles(include_deletes=False):
+        path = file.LocalPath()
+        # We are checking path, and all paths below up to root. E.g. if path is
+        # a/b/c, we start with path == "a/b/c", followed by "a/b" and "a".
+        while path:
+            if path in dependency_paths:
+                errors.append(
+                    output_api.PresubmitError(
+                        'You cannot place files tracked by Git inside a '
+                        'first-party DEPS dependency (deps).\n'
+                        f'Dependency: {path}\n'
+                        f'File: {file.LocalPath()}'))
+            path = _os.path.dirname(path)
+
+    return errors
+
+
 @functools.lru_cache(maxsize=None)
 def _ParseDeps(contents):
     """Simple helper for parsing DEPS files."""

tests/presubmit_canned_checks_test.py

@@ -7,14 +7,13 @@ import os.path
 import subprocess
 import sys
 import unittest
+from unittest import mock
 
 ROOT_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
 sys.path.insert(0, ROOT_DIR)
 
-from testing_support.presubmit_canned_checks_test_mocks import MockFile
-from testing_support.presubmit_canned_checks_test_mocks import (MockInputApi,
-                                                                MockOutputApi)
-from testing_support.presubmit_canned_checks_test_mocks import MockChange
+from testing_support.presubmit_canned_checks_test_mocks import (
+    MockFile, MockAffectedFile, MockInputApi, MockOutputApi, MockChange)
 
 import presubmit_canned_checks
 
@@ -441,5 +440,65 @@ class CheckUpdateOwnersFileReferences(unittest.TestCase):
         self.assertEqual(0, len(results))
 
 
+class CheckNoNewGitFilesAddedInDependenciesTest(unittest.TestCase):
+
+    @mock.patch('presubmit_canned_checks._readDeps')
+    def testNonNested(self, readDeps):
+        readDeps.return_value = '''deps = {
+      'src/foo': {'url': 'bar', 'condition': 'non_git_source'},
+      'src/components/foo/bar': {'url': 'bar', 'condition': 'non_git_source'},
+    }'''
+
+        input_api = MockInputApi()
+        input_api.files = [
+            MockFile('components/foo/file1.java', ['otherFunction']),
+            MockFile('components/foo/file2.java', ['hasSyncConsent']),
+            MockFile('chrome/foo/file3.java', ['canSyncFeatureStart']),
+            MockFile('chrome/foo/file4.java', ['isSyncFeatureEnabled']),
+            MockFile('chrome/foo/file5.java', ['isSyncFeatureActive']),
+        ]
+        results = presubmit_canned_checks.CheckNoNewGitFilesAddedInDependencies(
+            input_api, MockOutputApi())
+
+        self.assertEqual(0, len(results))
+
+    @mock.patch('presubmit_canned_checks._readDeps')
+    def testCollision(self, readDeps):
+        readDeps.return_value = '''deps = {
+      'src/foo': {'url': 'bar', 'condition': 'non_git_source'},
+      'src/baz': {'url': 'baz'},
+    }'''
+
+        input_api = MockInputApi()
+        input_api.files = [
+            MockAffectedFile('fo', 'content'),  # no conflict
+            MockAffectedFile('foo', 'content'),  # conflict
+            MockAffectedFile('foo/bar', 'content'),  # conflict
+            MockAffectedFile('baz/qux', 'content'),  # conflict, but ignored
+        ]
+        results = presubmit_canned_checks.CheckNoNewGitFilesAddedInDependencies(
+            input_api, MockOutputApi())
+
+        self.assertEqual(2, len(results))
+        self.assertIn('File: foo', str(results))
+        self.assertIn('File: foo/bar', str(results))
+
+    @mock.patch('presubmit_canned_checks._readDeps')
+    def testNoDeps(self, readDeps):
+        readDeps.return_value = ''  # Empty deps
+
+        input_api = MockInputApi()
+        input_api.files = [
+            MockAffectedFile('fo', 'content'),  # no conflict
+            MockAffectedFile('foo', 'content'),  # conflict
+            MockAffectedFile('foo/bar', 'content'),  # conflict
+            MockAffectedFile('baz/qux', 'content'),  # conflict, but ignored
+        ]
+        results = presubmit_canned_checks.CheckNoNewGitFilesAddedInDependencies(
+            input_api, MockOutputApi())
+
+        self.assertEqual(0, len(results))
+
+
 if __name__ == '__main__':
     unittest.main()