From ae5b505174ec0d06942fe647239b9b2584649f55 Mon Sep 17 00:00:00 2001 From: Mykola Semko Date: Tue, 28 Sep 2021 07:52:56 -0700 Subject: [PATCH] java_binary blocklist pattern refactoring Summary: ^ Reviewed By: IanChilds fbshipit-source-id: 938ece57420ddf14e94ff278b82c07543fe3bac6 --- .../facebook/buck/jvm/java/JavaBinary.java | 12 +++---- .../facebook/buck/util/PatternsMatcher.java | 33 ++++++++----------- .../buck/jvm/java/JavaBinaryTest.java | 2 +- .../buck/util/PatternsMatcherTest.java | 33 +++++++++---------- 4 files changed, 37 insertions(+), 43 deletions(-) diff --git a/src/com/facebook/buck/jvm/java/JavaBinary.java b/src/com/facebook/buck/jvm/java/JavaBinary.java index 6c67a265e91..214dca1039c 100644 --- a/src/com/facebook/buck/jvm/java/JavaBinary.java +++ b/src/com/facebook/buck/jvm/java/JavaBinary.java @@ -79,9 +79,9 @@ public class JavaBinary extends AbstractBuildRuleWithDeclaredAndExtraDeps @SuppressWarnings("PMD.UnusedPrivateField") @AddToRuleKey - private final ImmutableSet blacklist; + private final ImmutableSet blocklist; - private final PatternsMatcher blacklistPatternsMatcher; + private final PatternsMatcher blocklistPatternsMatcher; private final ImmutableSet transitiveClasspathDeps; private final ImmutableSet transitiveClasspaths; @@ -99,7 +99,7 @@ public JavaBinary( boolean mergeManifests, boolean disallowAllDuplicates, @Nullable Path metaInfDirectory, - ImmutableSet blacklist, + ImmutableSet blocklist, ImmutableSet transitiveClasspathDeps, ImmutableSet transitiveClasspaths, boolean cache, @@ -114,8 +114,8 @@ public JavaBinary( metaInfDirectory != null ? PathSourcePath.of(getProjectFilesystem(), metaInfDirectory) : null; - this.blacklist = blacklist; - blacklistPatternsMatcher = new PatternsMatcher(blacklist); + this.blocklist = blocklist; + this.blocklistPatternsMatcher = new PatternsMatcher(blocklist); this.transitiveClasspathDeps = transitiveClasspathDeps; this.transitiveClasspaths = transitiveClasspaths; this.cache = cache; @@ -181,7 +181,7 @@ public ImmutableList getBuildSteps( .setDuplicatesLogLevel(duplicatesLogLevel) .setRemoveEntryPredicate( entry -> - blacklistPatternsMatcher.substringMatches(((ZipEntry) entry).getName())) + blocklistPatternsMatcher.substringMatches(((ZipEntry) entry).getName())) .build()); commands.add(jar); diff --git a/src/com/facebook/buck/util/PatternsMatcher.java b/src/com/facebook/buck/util/PatternsMatcher.java index 00940f2008a..64a4deb8358 100644 --- a/src/com/facebook/buck/util/PatternsMatcher.java +++ b/src/com/facebook/buck/util/PatternsMatcher.java @@ -16,12 +16,12 @@ package com.facebook.buck.util; +import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Maps; -import java.util.Collection; import java.util.Map; +import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; /** * Helper class that keeps a list of compiled patterns and provides a method to check whether a @@ -34,36 +34,26 @@ public class PatternsMatcher { /** A pattern which matches no string */ public static final PatternsMatcher NONE = new PatternsMatcher(ImmutableSet.of(), false); - private final Collection patterns; + private final ImmutableSet patterns; /** True is like having a pattern {@code .*} in the pattern set */ private final boolean matchesAny; - private PatternsMatcher(Collection patterns, boolean matchesAny) { + private PatternsMatcher(ImmutableSet patterns, boolean matchesAny) { this.patterns = patterns; this.matchesAny = matchesAny; } - public PatternsMatcher(Collection rawPatterns) { - patterns = rawPatterns.stream().map(Pattern::compile).collect(Collectors.toList()); - matchesAny = false; + public PatternsMatcher(ImmutableCollection rawPatterns) { + this(rawPatterns.stream().map(Pattern::compile).collect(ImmutableSet.toImmutableSet()), false); } public PatternsMatcher(ImmutableSet compiledPatterns) { - patterns = compiledPatterns; - matchesAny = false; + this(compiledPatterns, false); } /** @return true if the given string matches some of the patterns */ public boolean matches(String string) { - if (matchesAny) { - return true; - } - for (Pattern pattern : patterns) { - if (pattern.matcher(string).matches()) { - return true; - } - } - return false; + return match(string, true); } /** @@ -71,11 +61,16 @@ public boolean matches(String string) { * patterns */ public boolean substringMatches(String string) { + return match(string, false); + } + + private boolean match(String string, boolean fullMatch) { if (matchesAny) { return true; } for (Pattern pattern : patterns) { - if (pattern.matcher(string).find()) { + Matcher matcher = pattern.matcher(string); + if (fullMatch ? matcher.matches() : matcher.find()) { return true; } } diff --git a/test/com/facebook/buck/jvm/java/JavaBinaryTest.java b/test/com/facebook/buck/jvm/java/JavaBinaryTest.java index e0b19b5f6bd..2db83713f9c 100644 --- a/test/com/facebook/buck/jvm/java/JavaBinaryTest.java +++ b/test/com/facebook/buck/jvm/java/JavaBinaryTest.java @@ -93,7 +93,7 @@ public void testGetExecutableCommand() { /* merge manifests */ true, false, null, - /* blacklist */ ImmutableSet.of(), + /* blocklist */ ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of(), /* cache */ true, diff --git a/test/com/facebook/buck/util/PatternsMatcherTest.java b/test/com/facebook/buck/util/PatternsMatcherTest.java index 17a6c61e4c0..5744a6b93b5 100644 --- a/test/com/facebook/buck/util/PatternsMatcherTest.java +++ b/test/com/facebook/buck/util/PatternsMatcherTest.java @@ -20,8 +20,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import java.util.Arrays; -import java.util.Collections; +import com.google.common.collect.ImmutableList; import java.util.Map; import java.util.TreeMap; import org.junit.Test; @@ -31,7 +30,7 @@ public class PatternsMatcherTest { @Test public void testMatchesPattern() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertTrue(patternsMatcher.matches("pattern")); assertTrue(patternsMatcher.matches("test_pattern")); @@ -42,7 +41,7 @@ public void testMatchesPattern() { @Test public void testMatchesAnyWithExactMatch() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertTrue(patternsMatcher.matches("test_pattern")); } @@ -50,56 +49,56 @@ public void testMatchesAnyWithExactMatch() { @Test public void testMatchesAnyWithWildcard() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertTrue(patternsMatcher.matches("pattern")); } @Test public void testDoesNotMatchPrefix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("test")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("test")); assertFalse(patternsMatcher.matches("test_pattern")); } @Test public void testMatchAnyWithNonMatchingPrefixReturnsFalse() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("test")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("test")); assertFalse(patternsMatcher.matches("test_pattern")); } @Test public void testSubstringMatchesPrefix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("test")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("test")); assertTrue(patternsMatcher.substringMatches("test_pattern")); } @Test public void testDoesNotMatchSuffix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("pattern")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("pattern")); assertFalse(patternsMatcher.matches("test_pattern")); } @Test public void testSubstringMatchesSuffix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("pattern")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("pattern")); assertTrue(patternsMatcher.substringMatches("test_pattern")); } @Test public void testDoesNotMatchInfix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("_")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("_")); assertFalse(patternsMatcher.matches("test_pattern")); } @Test public void testSubstringMatchesInfix() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.singletonList("_")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("_")); assertTrue(patternsMatcher.substringMatches("test_pattern")); } @@ -107,7 +106,7 @@ public void testSubstringMatchesInfix() { @Test public void testDoesNotMatchPattern() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertFalse(patternsMatcher.matches("wrong_pattern")); assertFalse(patternsMatcher.substringMatches("wrong_pat")); @@ -116,7 +115,7 @@ public void testDoesNotMatchPattern() { @Test public void testMatchesAnyDoesNotMatchPattern() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertFalse(patternsMatcher.matches("wrong_pattern")); } @@ -134,7 +133,7 @@ public void testMatchesMatchesEmptyPatterns() { @Test public void testHasPatterns() { PatternsMatcher patternsMatcher = - new PatternsMatcher(Arrays.asList("pattern.*", "test_pattern")); + new PatternsMatcher(ImmutableList.of("pattern.*", "test_pattern")); assertFalse(patternsMatcher.isMatchesAny()); assertFalse(patternsMatcher.isMatchesNone()); @@ -142,7 +141,7 @@ public void testHasPatterns() { @Test public void testHasNoPatterns() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Collections.emptyList()); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of()); assertTrue(patternsMatcher.isMatchesNone()); assertFalse(patternsMatcher.isMatchesAny()); @@ -166,7 +165,7 @@ public void testFilterMatchingMapEntriesWithEmptyPatterns() { @Test public void testFilterMatchingMapEntries() { - PatternsMatcher patternsMatcher = new PatternsMatcher(Arrays.asList("e1", "e2")); + PatternsMatcher patternsMatcher = new PatternsMatcher(ImmutableList.of("e1", "e2")); Map entries = new TreeMap() {