From 02b6d4cec8dd5400b008c4531cd099895cc6381c Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Thu, 17 Mar 2016 14:46:03 +1100 Subject: [PATCH 1/3] added support for --allow-duplicate-declarations flag --- src/com/google/common/css/JobDescription.java | 3 +++ .../common/css/JobDescriptionBuilder.java | 18 ++++++++++++++++-- .../ClosureCommandLineCompiler.java | 5 +++++ .../common/css/compiler/passes/PassRunner.java | 6 ++++-- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/com/google/common/css/JobDescription.java b/src/com/google/common/css/JobDescription.java index c42133c1..68eb99b2 100644 --- a/src/com/google/common/css/JobDescription.java +++ b/src/com/google/common/css/JobDescription.java @@ -48,6 +48,7 @@ public class JobDescription { public final boolean eliminateDeadStyles; public final boolean allowDefPropagation; public final boolean allowUnrecognizedFunctions; + public final boolean allowDuplicateDeclarations; public final Set allowedNonStandardFunctions; public final boolean allowUnrecognizedProperties; public final Set allowedUnrecognizedProperties; @@ -125,6 +126,7 @@ public enum SourceMapDetailLevel { ALL, DEFAULT } boolean swapLeftRightInUrl, boolean simplifyCss, boolean eliminateDeadStyles, boolean allowDefPropagation, boolean allowUnrecognizedFunctions, + boolean allowDuplicateDeclarations, Set allowedNonStandardFunctions, boolean allowUnrecognizedProperties, Set allowedUnrecognizedProperties, boolean allowUndefinedConstants, @@ -163,6 +165,7 @@ public enum SourceMapDetailLevel { ALL, DEFAULT } this.eliminateDeadStyles = eliminateDeadStyles; this.allowDefPropagation = allowDefPropagation; this.allowUnrecognizedFunctions = allowUnrecognizedFunctions; + this.allowDuplicateDeclarations = allowDuplicateDeclarations; this.allowedNonStandardFunctions = ImmutableSet.copyOf( allowedNonStandardFunctions); this.allowUnrecognizedProperties = allowUnrecognizedProperties; diff --git a/src/com/google/common/css/JobDescriptionBuilder.java b/src/com/google/common/css/JobDescriptionBuilder.java index 92fe03ca..df32300b 100644 --- a/src/com/google/common/css/JobDescriptionBuilder.java +++ b/src/com/google/common/css/JobDescriptionBuilder.java @@ -52,6 +52,7 @@ public class JobDescriptionBuilder { boolean eliminateDeadStyles; boolean allowDefPropagation; boolean allowUnrecognizedFunctions; + boolean allowDuplicateDeclarations; Set allowedNonStandardFunctions; boolean allowUnrecognizedProperties; Set allowedUnrecognizedProperties; @@ -89,6 +90,7 @@ public JobDescriptionBuilder() { this.eliminateDeadStyles = false; this.allowDefPropagation = false; this.allowUnrecognizedFunctions = false; + this.allowDuplicateDeclarations = false; this.allowedNonStandardFunctions = Sets.newHashSet(); this.allowUnrecognizedProperties = false; this.allowedUnrecognizedProperties = Sets.newHashSet(); @@ -125,6 +127,7 @@ public JobDescriptionBuilder copyFrom(JobDescription jobToCopy) { this.eliminateDeadStyles = jobToCopy.eliminateDeadStyles; this.allowDefPropagation = jobToCopy.allowDefPropagation; this.allowUnrecognizedFunctions = jobToCopy.allowUnrecognizedFunctions; + this.allowDuplicateDeclarations = jobToCopy.allowDuplicateDeclarations; this.allowedNonStandardFunctions = ImmutableSet.copyOf(jobToCopy.allowedNonStandardFunctions); this.allowUnrecognizedProperties = jobToCopy.allowUnrecognizedProperties; @@ -335,6 +338,16 @@ public JobDescriptionBuilder setAllowedNonStandardFunctions( return this; } + public JobDescriptionBuilder setAllowDuplicateDeclarations(boolean allow) { + checkJobIsNotAlreadyCreated(); + this.allowDuplicateDeclarations = allow; + return this; + } + + public JobDescriptionBuilder allowDuplicateDeclarations() { + return setAllowDuplicateDeclarations(true); + } + public JobDescriptionBuilder setAllowUnrecognizedProperties(boolean allow) { checkJobIsNotAlreadyCreated(); this.allowUnrecognizedProperties = allow; @@ -464,8 +477,9 @@ public JobDescription getJobDescription() { copyrightNotice, outputFormat, inputOrientation, outputOrientation, optimize, trueConditionNames, useInternalBidiFlipper, swapLtrRtlInUrl, swapLeftRightInUrl, simplifyCss, eliminateDeadStyles, - allowDefPropagation, allowUnrecognizedFunctions, allowedNonStandardFunctions, - allowUnrecognizedProperties, allowedUnrecognizedProperties, allowUndefinedConstants, + allowDefPropagation, allowUnrecognizedFunctions, allowDuplicateDeclarations, + allowedNonStandardFunctions, allowUnrecognizedProperties, + allowedUnrecognizedProperties, allowUndefinedConstants, allowMozDocument, vendor, allowKeyframes, allowWebkitKeyframes, processDependencies, allowedAtRules, cssRenamingPrefix, excludedClassesFromRenaming, diff --git a/src/com/google/common/css/compiler/commandline/ClosureCommandLineCompiler.java b/src/com/google/common/css/compiler/commandline/ClosureCommandLineCompiler.java index 2d4b22a8..733eccc8 100644 --- a/src/com/google/common/css/compiler/commandline/ClosureCommandLineCompiler.java +++ b/src/com/google/common/css/compiler/commandline/ClosureCommandLineCompiler.java @@ -153,6 +153,10 @@ static class Flags { "Allow unrecognized functions.") private boolean allowUnrecognizedFunctions = false; + @Option(name = "--allow-duplicate-declarations", usage = + "Allow duplicate declarations.") + private boolean allowDuplicateDeclarations = false; + @Option(name = "--allowed-non-standard-function", usage = "Specify a non-standard function to whitelist, like alpha()") private List allowedNonStandardFunctions = Lists.newArrayList(); @@ -223,6 +227,7 @@ JobDescription createJobDescription() { builder.setTrueConditionNames(trueConditions); builder.setAllowDefPropagation(allowDefPropagation); builder.setAllowUnrecognizedFunctions(allowUnrecognizedFunctions); + builder.setAllowDuplicateDeclarations(allowDuplicateDeclarations); builder.setAllowedNonStandardFunctions(allowedNonStandardFunctions); builder.setAllowedUnrecognizedProperties(allowedUnrecognizedProperties); builder.setAllowUnrecognizedProperties(allowUnrecognizedProperties); diff --git a/src/com/google/common/css/compiler/passes/PassRunner.java b/src/com/google/common/css/compiler/passes/PassRunner.java index 6dd0836d..44480b57 100644 --- a/src/com/google/common/css/compiler/passes/PassRunner.java +++ b/src/com/google/common/css/compiler/passes/PassRunner.java @@ -142,10 +142,12 @@ public void runPasses(CssTree cssTree) { new AbbreviatePositionalValues( cssTree.getMutatingVisitController()).runPass(); } - if (job.eliminateDeadStyles) { - // Report errors for duplicate declarations + // Report errors for duplicate declarations + if (job.allowDuplicateDeclarations) { new DisallowDuplicateDeclarations( cssTree.getVisitController(), errorManager).runPass(); + } + if (job.eliminateDeadStyles) { // Split rules by selector and declaration. new SplitRulesetNodes(cssTree.getMutatingVisitController()).runPass(); // Dead code elimination. From c1d5186fc04dd2f3c89a64f1101d975e26405bdc Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Thu, 17 Mar 2016 14:54:55 +1100 Subject: [PATCH 2/3] fixed minor bug with checking being inverted --- src/com/google/common/css/compiler/passes/PassRunner.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/com/google/common/css/compiler/passes/PassRunner.java b/src/com/google/common/css/compiler/passes/PassRunner.java index 44480b57..c73e5601 100644 --- a/src/com/google/common/css/compiler/passes/PassRunner.java +++ b/src/com/google/common/css/compiler/passes/PassRunner.java @@ -143,7 +143,7 @@ public void runPasses(CssTree cssTree) { cssTree.getMutatingVisitController()).runPass(); } // Report errors for duplicate declarations - if (job.allowDuplicateDeclarations) { + if (!job.allowDuplicateDeclarations) { new DisallowDuplicateDeclarations( cssTree.getVisitController(), errorManager).runPass(); } From 85d127c8de946f1b2fff2f0d5756f44c81329fd7 Mon Sep 17 00:00:00 2001 From: Bradley Mclain Date: Wed, 4 Oct 2017 16:27:22 +1100 Subject: [PATCH 3/3] adds test for allowDuplicateDeclarations defaulting to false --- .../commandline/ClosureCommandLineCompilerTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/com/google/common/css/compiler/commandline/ClosureCommandLineCompilerTest.java b/tests/com/google/common/css/compiler/commandline/ClosureCommandLineCompilerTest.java index a81a9ae7..f53b7519 100644 --- a/tests/com/google/common/css/compiler/commandline/ClosureCommandLineCompilerTest.java +++ b/tests/com/google/common/css/compiler/commandline/ClosureCommandLineCompilerTest.java @@ -76,6 +76,15 @@ public void testAllowDefPropagationDefaultsToTrue() throws Exception { @Test + public void testAllowDuplicateDeclarationsDefaultsToFalse() throws Exception { + ClosureCommandLineCompiler.Flags flags = + ClosureCommandLineCompiler.parseArgs(new String[] {"/dev/null"}, EXIT_CODE_HANDLER); + JobDescription jobDescription = flags.createJobDescription(); + assertThat(jobDescription.allowDuplicateDeclarations).isFalse(); + } + + @Test + public void testEmptyImportBlocks() throws Exception { // See b/29995881 ErrorManager errorManager = new NewFunctionalTestBase.TestErrorManager(new String[0]);