Fix path pattern matching to work also for gitattributes

Path pattern matching for attribute rules is different than matching
for excluded files.

The first difference concerns patterns without slashes. For
gitattributes those must match on the last component only, not on
any earlier segment. This is true also for directory-only patterns.

The second difference concerns directory-only patterns. Those also
must not match on a prefix or segment except the last one. They do
not apply recursively to all files beneath.

And third, matches only on a prefix must match for gitattributes
only if the last matcher was "/**".

Add a new parameter for such path matching to IMatcher.matches() and
pass it through as appropriate (false for gitignore, true for
gitattributes). As far as gitignore is concerned, there is no change.

New tests have been added, and some existing attribute matching tests
have been fixed since they operated on wrong assumptions.

Bug: 508568
Change-Id: Ie825dc2cac8a85a72a7eeb0abb888f3193d21dd2
Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>

4 files changed, 107 insertions, 59 deletions

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java
index 61f7b83400..5b184cb19f 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/IMatcher.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2014, Andrey Loskutov <loskutov@gmx.de>
+ * Copyright (C) 2014, 2017 Andrey Loskutov <loskutov@gmx.de>
  * and other copyright owners as documented in the project's IP log.
  *
  * This program and the accompanying materials are made available
@@ -52,7 +52,8 @@ public interface IMatcher {
 	 */
 	public static final IMatcher NO_MATCH = new IMatcher() {
 		@Override
-		public boolean matches(String path, boolean assumeDirectory) {
+		public boolean matches(String path, boolean assumeDirectory,
+				boolean pathMatch) {
 			return false;
 		}
 
@@ -71,9 +72,14 @@ public interface IMatcher {
 	 * @param assumeDirectory
 	 *            true to assume this path as directory (even if it doesn't end
 	 *            with a slash)
+	 * @param pathMatch
+	 *            {@code true} if the match is for the full path: prefix-only
+	 *            matches are not allowed, and {@link NameMatcher}s must match
+	 *            only the last component (if they can -- they may not, if they
+	 *            are anchored at the beginning)
 	 * @return true if this matcher pattern matches given string
 	 */
-	boolean matches(String path, boolean assumeDirectory);
+	boolean matches(String path, boolean assumeDirectory, boolean pathMatch);
 
 	/**
 	 * Matches only part of given string
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java
index 00651237db..9667837a41 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/NameMatcher.java
@@ -64,26 +64,59 @@ public class NameMatcher extends AbstractMatcher {
 			pattern = Strings.deleteBackslash(pattern);
 		}
 		beginning = pattern.length() == 0 ? false : pattern.charAt(0) == slash;
-		if (!beginning)
+		if (!beginning) {
 			this.subPattern = pattern;
-		else
+		} else {
 			this.subPattern = pattern.substring(1);
+		}
 	}
 
 	@Override
-	public boolean matches(String path, boolean assumeDirectory) {
-		int end = 0;
-		int firstChar = 0;
-		do {
-			firstChar = getFirstNotSlash(path, end);
-			end = getFirstSlash(path, firstChar);
-			boolean match = matches(path, firstChar, end, assumeDirectory);
-			if (match)
+	public boolean matches(String path, boolean assumeDirectory,
+			boolean pathMatch) {
+		// A NameMatcher's pattern does not contain a slash.
+		int start = 0;
+		int stop = path.length();
+		if (stop > 0 && path.charAt(0) == slash) {
+			start++;
+		}
+		if (pathMatch) {
+			// Can match only after the last slash
+			int lastSlash = path.lastIndexOf(slash, stop - 1);
+			if (lastSlash == stop - 1) {
+				// Skip trailing slash
+				lastSlash = path.lastIndexOf(slash, lastSlash - 1);
+				stop--;
+			}
+			boolean match;
+			if (lastSlash < start) {
+				match = matches(path, start, stop, assumeDirectory);
+			} else {
+				// Can't match if the path contains a slash if the pattern is
+				// anchored at the beginning
+				match = !beginning
+						&& matches(path, lastSlash + 1, stop, assumeDirectory);
+			}
+			if (match && dirOnly) {
+				match = assumeDirectory;
+			}
+			return match;
+		}
+		while (start < stop) {
+			int end = path.indexOf(slash, start);
+			if (end < 0) {
+				end = stop;
+			}
+			if (end > start && matches(path, start, end, assumeDirectory)) {
 				// make sure the directory matches: either if we are done with
 				// segment and there is next one, or if the directory is assumed
-				return !dirOnly ? true : (end > 0 && end != path.length())
-						|| assumeDirectory;
-		} while (!beginning && end != path.length());
+				return !dirOnly || assumeDirectory || end < stop;
+			}
+			if (beginning) {
+				break;
+			}
+			start = end + 1;
+		}
 		return false;
 	}
 
@@ -92,25 +125,18 @@ public class NameMatcher extends AbstractMatcher {
 			boolean assumeDirectory) {
 		// faster local access, same as in string.indexOf()
 		String s = subPattern;
-		if (s.length() != (endExcl - startIncl))
+		int length = s.length();
+		if (length != (endExcl - startIncl)) {
 			return false;
-		for (int i = 0; i < s.length(); i++) {
+		}
+		for (int i = 0; i < length; i++) {
 			char c1 = s.charAt(i);
 			char c2 = segment.charAt(i + startIncl);
-			if (c1 != c2)
+			if (c1 != c2) {
 				return false;
+			}
 		}
 		return true;
 	}
 
-	private int getFirstNotSlash(String s, int start) {
-		int slashIdx = s.indexOf(slash, start);
-		return slashIdx == start ? start + 1 : start;
-	}
-
-	private int getFirstSlash(String s, int start) {
-		int slashIdx = s.indexOf(slash, start);
-		return slashIdx == -1 ? s.length() : slashIdx;
-	}
-
 }
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
index ce9ad80fb0..85073ecfb2 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/PathMatcher.java
@@ -52,7 +52,6 @@ import java.util.ArrayList;
 import java.util.List;
 
 import org.eclipse.jgit.errors.InvalidPatternException;
-import org.eclipse.jgit.ignore.FastIgnoreRule;
 import org.eclipse.jgit.ignore.internal.Strings.PatternState;
 
 /**
@@ -68,9 +67,10 @@ public class PathMatcher extends AbstractMatcher {
 
 	private final char slash;
 
-	private boolean beginning;
+	private final boolean beginning;
 
-	PathMatcher(String pattern, Character pathSeparator, boolean dirOnly)
+	private PathMatcher(String pattern, Character pathSeparator,
+			boolean dirOnly)
 			throws InvalidPatternException {
 		super(pattern, dirOnly);
 		slash = getPathSeparator(pathSeparator);
@@ -87,7 +87,7 @@ public class PathMatcher extends AbstractMatcher {
 				&& count(path, slash, true) > 0;
 	}
 
-	static private List<IMatcher> createMatchers(List<String> segments,
+	private static List<IMatcher> createMatchers(List<String> segments,
 			Character pathSeparator, boolean dirOnly)
 			throws InvalidPatternException {
 		List<IMatcher> matchers = new ArrayList<>(segments.size());
@@ -171,10 +171,12 @@ public class PathMatcher extends AbstractMatcher {
 	}
 
 	@Override
-	public boolean matches(String path, boolean assumeDirectory) {
-		if (matchers == null)
-			return simpleMatch(path, assumeDirectory);
-		return iterate(path, 0, path.length(), assumeDirectory);
+	public boolean matches(String path, boolean assumeDirectory,
+			boolean pathMatch) {
+		if (matchers == null) {
+			return simpleMatch(path, assumeDirectory, pathMatch);
+		}
+		return iterate(path, 0, path.length(), assumeDirectory, pathMatch);
 	}
 
 	/*
@@ -182,31 +184,31 @@ public class PathMatcher extends AbstractMatcher {
 	 * wildcards or single segments (mean: this is multi-segment path which must
 	 * be at the beginning of the another string)
 	 */
-	private boolean simpleMatch(String path, boolean assumeDirectory) {
+	private boolean simpleMatch(String path, boolean assumeDirectory,
+			boolean pathMatch) {
 		boolean hasSlash = path.indexOf(slash) == 0;
-		if (beginning && !hasSlash)
+		if (beginning && !hasSlash) {
 			path = slash + path;
-
-		if (!beginning && hasSlash)
+		}
+		if (!beginning && hasSlash) {
 			path = path.substring(1);
-
-		if (path.equals(pattern))
-			// Exact match
-			if (dirOnly && !assumeDirectory)
-				// Directory expectations not met
-				return false;
-			else
-				// Directory expectations met
-				return true;
-
+		}
+		if (path.equals(pattern)) {
+			// Exact match: must meet directory expectations
+			return !dirOnly || assumeDirectory;
+		}
 		/*
 		 * Add slashes for startsWith check. This avoids matching e.g.
 		 * "/src/new" to /src/newfile" but allows "/src/new" to match
 		 * "/src/new/newfile", as is the git standard
 		 */
-		if (path.startsWith(pattern + FastIgnoreRule.PATH_SEPARATOR))
+		String prefix = pattern + slash;
+		if (pathMatch) {
+			return path.equals(prefix) && (!dirOnly || assumeDirectory);
+		}
+		if (path.startsWith(prefix)) {
 			return true;
-
+		}
 		return false;
 	}
 
@@ -217,8 +219,8 @@ public class PathMatcher extends AbstractMatcher {
 				"Path matcher works only on entire paths"); //$NON-NLS-1$
 	}
 
-	boolean iterate(final String path, final int startIncl, final int endExcl,
-			boolean assumeDirectory) {
+	private boolean iterate(final String path, final int startIncl,
+			final int endExcl, boolean assumeDirectory, boolean pathMatch) {
 		int matcher = 0;
 		int right = startIncl;
 		boolean match = false;
@@ -256,14 +258,26 @@ public class PathMatcher extends AbstractMatcher {
 				continue;
 			}
 			if (match) {
-				if (matchers.get(matcher) == WILD) {
+				boolean wasWild = matchers.get(matcher) == WILD;
+				if (wasWild) {
 					lastWildmatch = matcher;
 					// ** can match *nothing*: a/**/b match also a/b
 					right = left - 1;
 				}
 				matcher++;
 				if (matcher == matchers.size()) {
-					return true;
+					// We had a prefix match here.
+					if (!pathMatch) {
+						return true;
+					} else {
+						if (right == endExcl - 1) {
+							// Extra slash at the end: actually a full match.
+							// Must meet directory expectations
+							return !dirOnly || assumeDirectory;
+						}
+						// Prefix matches only if pattern ended with /**
+						return wasWild;
+					}
 				}
 			} else if (lastWildmatch != -1) {
 				matcher = lastWildmatch + 1;
@@ -274,7 +288,8 @@ public class PathMatcher extends AbstractMatcher {
 		}
 	}
 
-	boolean matches(int matcherIdx, String path, int startIncl, int endExcl,
+	private boolean matches(int matcherIdx, String path, int startIncl,
+			int endExcl,
 			boolean assumeDirectory) {
 		IMatcher matcher = matchers.get(matcherIdx);
 		return matcher.matches(path, startIncl, endExcl, assumeDirectory);
diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java
index 93ea13c726..363b3cee84 100644
--- a/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/ignore/internal/WildMatcher.java
@@ -62,7 +62,8 @@ public final class WildMatcher extends AbstractMatcher {
 	}
 
 	@Override
-	public final boolean matches(String path, boolean assumeDirectory) {
+	public final boolean matches(String path, boolean assumeDirectory,
+			boolean pathMatch) {
 		return true;
 	}