diff --git a/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs b/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs index 9a870666fb2..1b1470fb29b 100644 --- a/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs +++ b/tools/code-owners-parser/CodeOwnersParser/MatchedCodeOwnerEntry.cs @@ -89,37 +89,11 @@ private static bool Matches(string targetPath, CodeOwnerEntry entry) { string codeownersPath = entry.PathExpression; - targetPath = NormalizeTargetPath(targetPath, codeownersPath); - - Regex regex = ConvertToRegex(codeownersPath); + Regex regex = ConvertToRegex(targetPath, codeownersPath); return regex.IsMatch(targetPath); } - private static string NormalizeTargetPath(string targetPath, string codeownersPath) - { - // If the considered CODEOWNERS path ends with "/", it means we can - // assume targetPath also is a path to directory. - // - // This works in all 3 cases, which are: - // - // 1. The targetPath is the same as the CODEOWNERS path, except - // the targetPath doesn't have "/" at the end. If so, - // it might be a path to a file or directory, - // but the exact path match with CODEOWNERS path and our validation - // guarantees it is an existing directory, hence we can append "/". - // - // 2. The targetPath is a prefix path of CODEOWNERS path. In such case - // there won't be a match, and appending "/" won't change that. - // - // 3. The CODEOWNERS path is a prefix path of targetPath. In such case - // there will be a match, and appending "/" won't change that. - if (codeownersPath.EndsWith("/") && !targetPath.EndsWith("/")) - targetPath += "/"; - - return targetPath; - } - - private static Regex ConvertToRegex(string codeownersPath) + private static Regex ConvertToRegex(string targetPath, string codeownersPath) { codeownersPath = ConvertPathIfInvalid(codeownersPath); @@ -132,7 +106,10 @@ private static Regex ConvertToRegex(string codeownersPath) pattern = Regex.Escape(pattern); - pattern = AddStringAnchors(pattern); + // Denote that all paths are absolute by pre-pending "beginning of string" symbol. + pattern = "^" + pattern; + + pattern = SetPatternSuffix(targetPath, pattern); // Note that the "/**/" case is implicitly covered by "**/". pattern = pattern.Replace("_DOUBLE_STAR_/", "(.*)"); @@ -141,25 +118,12 @@ private static Regex ConvertToRegex(string codeownersPath) // This case is necessary to cover inline **, e.g. "/a**b/". pattern = pattern.Replace("_DOUBLE_STAR_", "(.*)"); pattern = pattern.Replace("_SINGLE_STAR_", "([^/]*)"); - return new Regex(pattern); - } - private static string ConvertPathIfInvalid(string codeownersPath) - { - // CODEOWNERS paths that do not start with "/" are relative and considered invalid. - // However, here we handle such cases to accomodate for parsing CODEOWNERS file - // paths that somehow slipped through validation. We do so by instead treating - // such paths as if they were absolute to repository root, i.e. starting with "/". - if (!IsCodeOwnersPathValid(codeownersPath)) - codeownersPath = "/" + codeownersPath; - return codeownersPath; + return new Regex(pattern); } - private static string AddStringAnchors(string pattern) + private static string SetPatternSuffix(string targetPath, string pattern) { - // Denote that all paths are absolute by pre-pending "beginning of string" symbol. - pattern = "^" + pattern; - // Lack of slash at the end denotes the path is a path to a file, // per our validation logic. // Note we assume this is path to a file even if the path is invalid, @@ -170,10 +134,44 @@ private static string AddStringAnchors(string pattern) // not a substring, as we are dealing with a file. pattern += "$"; } + // If the CODEOWNERS pattern is matching only against directories, + // but the targetPath may not be a directory, we need to trim + // the "/" from the pattern to ensure match is present. + // + // To illustrate this, consider following cases: + // + // targetPath: /a , /a*/ + // pattern: /a/ , /abc + // + // Without trimming pattern to be "/a" and "/a*" respectively, + // these wouldn't match, but they should. + // + // On the other hand, trimming the suffix "/" when it is not + // necessary won't lead to issues. E.g.: + // + // targetPath: /a/b + // pattern: /a/ + // + // Here we still have a prefix match even if we trim pattern to "/a". + else if (pattern.EndsWith("/") && !targetPath.EndsWith("/")) + { + pattern = pattern.TrimEnd('/'); + } return pattern; } + private static string ConvertPathIfInvalid(string codeownersPath) + { + // CODEOWNERS paths that do not start with "/" are relative and considered invalid. + // However, here we handle such cases to accomodate for parsing CODEOWNERS file + // paths that somehow slipped through validation. We do so by instead treating + // such paths as if they were absolute to repository root, i.e. starting with "/". + if (!IsCodeOwnersPathValid(codeownersPath)) + codeownersPath = "/" + codeownersPath; + return codeownersPath; + } + /// /// The entry is valid if it obeys following conditions: /// - The Value was obtained with a call to