Resolve excluded ancestor directories before descendant negations

CHANGES.rst

@@ -20,6 +20,8 @@ New features:
 
 Bug fixes:
 
+- Resolve excluded ancestor directories before descendant negations in all GitIgnoreSpec backends, and report the blocking pattern index.
+
 - Honor `on_error` when opening a directory fails during tree traversal.
 
 - `Pull #123`_: Ignore invalid gitignore bracket ranges for `GitIgnoreSpec`.

README.rst

@@ -93,6 +93,13 @@ handles these cases to more closely replicate Git's behavior::
 You do not specify the style of pattern for ``GitIgnoreSpec`` because it should
 always use ``GitIgnoreSpecPattern`` internally.
 
+A negated file pattern cannot re-include a file while any of its parent
+directories remains excluded. For example, ``dir/**`` followed by
+``!dir/**/file`` re-includes ``dir/file``, but leaves ``dir/nested/file`` ignored
+because ``dir/nested/`` is still excluded. Re-include the necessary parent
+directories as well. ``GitIgnoreSpec.check_file()`` reports the pattern index
+that excludes the first blocking ancestor in this situation.
+
 
 Performance
 -----------

pathspec/_backends/hyperscan/gitignore.py

@@ -204,8 +204,13 @@ def _match(
 		"""
 		Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is
 		whether to ask if an ancestor directory of *file* is excluded; see
-		:meth:`_ancestor_excluded` for why it can be skipped.
+		:meth:`_excluded_ancestor_index` for why it can be skipped.
 		"""
+		if check_ancestors:
+			ancestor_index = self._excluded_ancestor_index(file)
+			if ancestor_index is not None:
+				return (True, ancestor_index)
+
 		# NOTICE: According to benchmarking, a method callback is 13% faster than
 		# using a closure here.
 		db = self._db
@@ -218,9 +223,7 @@ def _match(
 		db.scan(file.encode('utf8'), match_event_handler=self.__on_match)
 
 		dir_include, dir_index, file_include, file_index = self._out
-		if dir_include and check_ancestors and self._ancestor_excluded(file):
-			out_include, out_index = dir_include, dir_index
-		elif file_include is not None:
+		if file_include is not None:
 			out_include, out_index = file_include, file_index
 		elif dir_include:
 			# An ancestor matched an exclude pattern, but the spec as a whole
@@ -231,10 +234,10 @@ def _match(
 
 		return (out_include, out_index if out_index != -1 else None)
 
-	def _ancestor_excluded(self, file: str) -> bool:
+	def _excluded_ancestor_index(self, file: str) -> Optional[int]:
 		"""
-		Whether any strict ancestor directory of *file* is excluded. Git stops
-		descending at the first excluded directory, so the ancestors are asked
+		Return the pattern index excluding the first strict ancestor, or None.
+		Git stops descending at the first excluded directory, so ancestors are asked
 		outermost first, each as a directory query (trailing slash included).
 		By the time an ancestor is asked, every ancestor above it is known not to
 		be excluded, so it is matched without checking its own ancestors again;
@@ -243,14 +246,14 @@ def _ancestor_excluded(self, file: str) -> bool:
 		"""
 		index = file.find('/')
 		while index != -1 and index + 1 < len(file):
-			ancestor_include, _ancestor_index = self._match(
+			ancestor_include, ancestor_index = self._match(
 				file[:index + 1], check_ancestors=False,
 			)
 			if ancestor_include:
-				return True
+				return ancestor_index
 			index = file.find('/', index + 1)
 
-		return False
+		return None
 
 
 	@override

pathspec/_backends/re2/gitignore.py

@@ -153,8 +153,13 @@ def _match(
 		"""
 		Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is
 		whether to ask if an ancestor directory of *file* is excluded; see
-		:meth:`_ancestor_excluded` for why it can be skipped.
+		:meth:`_excluded_ancestor_index` for why it can be skipped.
 		"""
+		if check_ancestors:
+			ancestor_index = self._excluded_ancestor_index(file)
+			if ancestor_index is not None:
+				return (True, ancestor_index)
+
 		# Find best match.
 		match_ids: Optional[list[int]] = self._set.Match(file)  # type: ignore[assignment]
 		if not match_ids:
@@ -186,9 +191,7 @@ def _match(
 				file_index = index
 
 		assert dir_index != -1 or file_index != -1, (dir_index, file_index)
-		if dir_include and check_ancestors and self._ancestor_excluded(file):
-			return (dir_include, dir_index)
-		elif file_include is not None:
+		if file_include is not None:
 			return (file_include, file_index)
 		elif dir_include:
 			# An ancestor matched an exclude pattern, but the spec as a whole
@@ -197,10 +200,10 @@ def _match(
 		else:
 			return (dir_include, dir_index)
 
-	def _ancestor_excluded(self, file: str) -> bool:
+	def _excluded_ancestor_index(self, file: str) -> Optional[int]:
 		"""
-		Whether any strict ancestor directory of *file* is excluded. Git stops
-		descending at the first excluded directory, so the ancestors are asked
+		Return the pattern index excluding the first strict ancestor, or None.
+		Git stops descending at the first excluded directory, so ancestors are asked
 		outermost first, each as a directory query (trailing slash included).
 		By the time an ancestor is asked, every ancestor above it is known not to
 		be excluded, so it is matched without checking its own ancestors again;
@@ -209,12 +212,11 @@ def _ancestor_excluded(self, file: str) -> bool:
 		"""
 		index = file.find('/')
 		while index != -1 and index + 1 < len(file):
-			ancestor_include, _ancestor_index = self._match(
+			ancestor_include, ancestor_index = self._match(
 				file[:index + 1], check_ancestors=False,
 			)
 			if ancestor_include:
-				return True
+				return ancestor_index
 			index = file.find('/', index + 1)
 
-		return False
-
+		return None

pathspec/_backends/simple/gitignore.py

@@ -70,8 +70,13 @@ def _match(
 		"""
 		Implements :meth:`match_file`. *check_ancestors* (:class:`bool`) is
 		whether to ask if an ancestor directory of *file* is excluded; see
-		:meth:`_ancestor_excluded` for why it can be skipped.
+		:meth:`_excluded_ancestor_index` for why it can be skipped.
 		"""
+		if check_ancestors:
+			ancestor_index = self._excluded_ancestor_index(file)
+			if ancestor_index is not None:
+				return (True, ancestor_index)
+
 		is_reversed = self._is_reversed
 
 		# Resolve the ancestor directory and the file separately: a file negation
@@ -112,9 +117,7 @@ def _match(
 					file_include = include
 					file_index = index
 
-		if dir_include and check_ancestors and self._ancestor_excluded(file):
-			return (dir_include, dir_index)
-		elif file_include is not None:
+		if file_include is not None:
 			return (file_include, file_index)
 		elif dir_include:
 			# An ancestor matched an exclude pattern, but the spec as a whole
@@ -123,10 +126,10 @@ def _match(
 		else:
 			return (dir_include, dir_index)
 
-	def _ancestor_excluded(self, file: str) -> bool:
+	def _excluded_ancestor_index(self, file: str) -> Optional[int]:
 		"""
-		Whether any strict ancestor directory of *file* is excluded. Git stops
-		descending at the first excluded directory, so the ancestors are asked
+		Return the pattern index excluding the first strict ancestor, or None.
+		Git stops descending at the first excluded directory, so ancestors are asked
 		outermost first, each as a directory query (trailing slash included).
 		By the time an ancestor is asked, every ancestor above it is known not to
 		be excluded, so it is matched without checking its own ancestors again;
@@ -135,12 +138,11 @@ def _ancestor_excluded(self, file: str) -> bool:
 		"""
 		index = file.find('/')
 		while index != -1 and index + 1 < len(file):
-			ancestor_include, _ancestor_index = self._match(
+			ancestor_include, ancestor_index = self._match(
 				file[:index + 1], check_ancestors=False,
 			)
 			if ancestor_include:
-				return True
+				return ancestor_index
 			index = file.find('/', index + 1)
 
-		return False
-
+		return None

tests/test_09_excluded_ancestors.py

@@ -0,0 +1,101 @@
+"""
+Tests descendant negations when a strict ancestor remains excluded.
+"""
+
+from unittest import TestCase
+
+from .test_06_gitignore import GitIgnoreSpecMixin
+
+
+class ExcludedAncestorTest(GitIgnoreSpecMixin, TestCase):
+	"""
+	Tests Git's rule that a file cannot be re-included under an excluded directory.
+	"""
+
+	def test_globstar_descendant_negation(self):
+		"""
+		A recursive file negation does not open the directories excluded by dir/**.
+		"""
+		for begin in self.parameterize_from_lines(['dir/**', '!dir/**/file']):
+			with begin() as spec:
+				self.assertEqual(spec.check_file('dir/file').include, False)
+				for file in ('dir/nested/file', 'dir/a/b/c/file', 'dir/nested/'):
+					with self.subTest(file=file):
+						result = spec.check_file(file)
+						self.assertEqual((result.include, result.index), (True, 0))
+				self.assertIsNone(spec.check_file('dir/').include)
+				self.assertIsNone(spec.check_file('elsewhere/file').include)
+
+	def test_match_all_file_negation(self):
+		"""
+		A match-all shortcut also excludes parent directories before file negations.
+		"""
+		for begin in self.parameterize_from_lines(['*', '!**/a.py']):
+			with begin() as spec:
+				self.assertEqual(spec.check_file('a.py').include, False)
+				for file in ('dir/a.py', 'dir/nested/a.py'):
+					with self.subTest(file=file):
+						result = spec.check_file(file)
+						self.assertEqual((result.include, result.index), (True, 0))
+
+	def test_parent_negation_does_not_open_all_descendants(self):
+		"""
+		Re-including one directory leaves its still-excluded nested directories closed.
+		"""
+		for begin in self.parameterize_from_lines(['*/', '!src/']):
+			with begin() as spec:
+				self.assertFalse(spec.match_file('src/a.txt'))
+				for file in ('src/nested/file', 'src/a/b/file', 'src/nested/'):
+					with self.subTest(file=file):
+						result = spec.check_file(file)
+						self.assertEqual((result.include, result.index), (True, 0))
+
+	def test_reinclude_each_ancestor(self):
+		"""
+		A negated leaf becomes reachable only when each excluded ancestor is reopened.
+		"""
+		lines = ['dir/**', '!dir/a/', '!dir/**/file']
+		for begin in self.parameterize_from_lines(lines):
+			with begin() as spec:
+				self.assertFalse(spec.match_file('dir/file'))
+				self.assertFalse(spec.match_file('dir/a/file'))
+				self.assertTrue(spec.match_file('dir/a/b/file'))
+				self.assertTrue(spec.match_file('dir/a/b/c/file'))
+
+		for begin in self.parameterize_from_lines(lines + ['!dir/a/b/']):
+			with begin() as spec:
+				self.assertFalse(spec.match_file('dir/a/b/file'))
+				self.assertTrue(spec.match_file('dir/a/b/c/file'))
+
+		for begin in self.parameterize_from_lines(lines + ['!dir/a/b/', '!dir/a/b/c/']):
+			with begin() as spec:
+				self.assertFalse(spec.match_file('dir/a/b/c/file'))
+
+	def test_first_blocking_ancestor_index(self):
+		"""
+		The reported index belongs to the first directory preventing traversal.
+		"""
+		for begin in self.parameterize_from_lines([
+			'dir/', 'dir/nested/', '!dir/nested/file',
+		]):
+			with begin() as spec:
+				for file in ('dir/nested/file', 'dir/nested/'):
+					result = spec.check_file(file)
+					self.assertEqual((result.include, result.index), (True, 0))
+
+		for begin in self.parameterize_from_lines([
+			'dir/', 'dir/nested/', '!dir/', '!dir/nested/file',
+		]):
+			with begin() as spec:
+				result = spec.check_file('dir/nested/file')
+				self.assertEqual((result.include, result.index), (True, 1))
+
+	def test_only_negations_and_unmatched_files(self):
+		"""
+		No implicit exclusion is introduced by ancestor inspection.
+		"""
+		for begin in self.parameterize_from_lines(['!src/', '!**/a.py']):
+			with begin() as spec:
+				self.assertFalse(spec.match_file('src/nested/a.py'))
+				self.assertFalse(spec.match_file('src/nested/b.py'))
+				self.assertIsNone(spec.check_file('elsewhere/file').include)