From 1bdf5f896398da4e035e0d547c26b1c5c12b2c53 Mon Sep 17 00:00:00 2001 From: Ian Flanigan Date: Tue, 29 Mar 2016 11:47:28 -0700 Subject: [PATCH] Allow conditional statements inside rulesets, including mixins. This change allows for conditionally including properties within a ruleset or mixin, like so: @defmixin foo() { @if (COND) { bar: baz } } Previously, this would cause an IllegalStateException in the @defmixin case from the precondition in CssConditionalRuleNode or an unknown @ rule in the ruleset case. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=118486801 --- .../compiler/ast/CssConditionalRuleNode.java | 7 +++--- .../compiler/ast/CssDeclarationBlockNode.java | 5 +++-- .../css/compiler/ast/CssTreeBuilder.java | 6 ++--- .../compiler/ast/DefaultVisitController.java | 2 +- .../passes/CreateConditionalNodes.java | 9 +++----- .../css/compiler/passes/PassRunner.java | 9 ++++---- .../passes/CreateConditionalNodesTest.java | 22 +++++++++++++++++++ 7 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/com/google/common/css/compiler/ast/CssConditionalRuleNode.java b/src/com/google/common/css/compiler/ast/CssConditionalRuleNode.java index 20d2acf8..e56defe1 100644 --- a/src/com/google/common/css/compiler/ast/CssConditionalRuleNode.java +++ b/src/com/google/common/css/compiler/ast/CssConditionalRuleNode.java @@ -47,7 +47,7 @@ public CssConditionalRuleNode(Type type, CssLiteralNode name) { */ public CssConditionalRuleNode(Type type, CssLiteralNode name, @Nullable CssBooleanExpressionNode condition, - CssBlockNode block) { + CssAbstractBlockNode block) { super(type, name, block); Preconditions.checkArgument(this.getType().isConditional()); if (condition != null) { @@ -98,9 +98,8 @@ void setCondition(CssBooleanExpressionNode condition) { } @Override - public CssBlockNode getBlock() { - // This type is ensured by the constructor. - return (CssBlockNode) super.getBlock(); + public CssAbstractBlockNode getBlock() { + return super.getBlock(); } // TODO(user): Make sure that the parameters list is made up of a single diff --git a/src/com/google/common/css/compiler/ast/CssDeclarationBlockNode.java b/src/com/google/common/css/compiler/ast/CssDeclarationBlockNode.java index 005b7d22..e284677a 100644 --- a/src/com/google/common/css/compiler/ast/CssDeclarationBlockNode.java +++ b/src/com/google/common/css/compiler/ast/CssDeclarationBlockNode.java @@ -19,7 +19,8 @@ import com.google.common.collect.ImmutableList; /** - * A {@link CssAbstractBlockNode} that contains only declarations and @-rules. + * A {@link CssAbstractBlockNode} that contains only declarations, @-rules, + * and conditional blocks. * * @author fbenz@google.com (Florian Benz) */ @@ -27,7 +28,7 @@ public class CssDeclarationBlockNode extends CssAbstractBlockNode { private static final ImmutableList> VALID_NODE_CLASSES = ImmutableList.of( - CssDeclarationNode.class, CssAtRuleNode.class); + CssDeclarationNode.class, CssAtRuleNode.class, CssConditionalBlockNode.class); public CssDeclarationBlockNode() { super(true, VALID_NODE_CLASSES); diff --git a/src/com/google/common/css/compiler/ast/CssTreeBuilder.java b/src/com/google/common/css/compiler/ast/CssTreeBuilder.java index 9866e9dd..9176f120 100644 --- a/src/com/google/common/css/compiler/ast/CssTreeBuilder.java +++ b/src/com/google/common/css/compiler/ast/CssTreeBuilder.java @@ -61,7 +61,7 @@ enum State { private boolean treeIsConstructed = false; // TODO(user): Use Collections.asLifoQueue(new ArrayDeque()) for openBlocks - private List openBlocks = null; + private List openBlocks = null; private List openConditionalBlocks = null; private CssDeclarationBlockNode declarationBlock = null; private CssDeclarationNode declaration = null; @@ -114,11 +114,11 @@ private void startMainBody() { } } - private CssBlockNode getEnclosingBlock() { + private CssAbstractBlockNode getEnclosingBlock() { return openBlocks.get(openBlocks.size() - 1); } - private void pushEnclosingBlock(CssBlockNode block) { + private void pushEnclosingBlock(CssAbstractBlockNode block) { openBlocks.add(block); } diff --git a/src/com/google/common/css/compiler/ast/DefaultVisitController.java b/src/com/google/common/css/compiler/ast/DefaultVisitController.java index ba5ec4a5..4c277943 100644 --- a/src/com/google/common/css/compiler/ast/DefaultVisitController.java +++ b/src/com/google/common/css/compiler/ast/DefaultVisitController.java @@ -851,7 +851,7 @@ public void transitionToNextState() { @VisibleForTesting class VisitConditionalRuleChildrenState extends VisitReplaceChildrenState { - VisitConditionalRuleChildrenState(CssBlockNode block) { + VisitConditionalRuleChildrenState(CssAbstractBlockNode block) { super(block); } } diff --git a/src/com/google/common/css/compiler/passes/CreateConditionalNodes.java b/src/com/google/common/css/compiler/passes/CreateConditionalNodes.java index e764ca8c..8a4fd6f7 100644 --- a/src/com/google/common/css/compiler/passes/CreateConditionalNodes.java +++ b/src/com/google/common/css/compiler/passes/CreateConditionalNodes.java @@ -16,11 +16,10 @@ package com.google.common.css.compiler.passes; -import com.google.common.base.Preconditions; import com.google.common.collect.Lists; import com.google.common.css.SourceCodeLocation; +import com.google.common.css.compiler.ast.CssAbstractBlockNode; import com.google.common.css.compiler.ast.CssAtRuleNode; -import com.google.common.css.compiler.ast.CssBlockNode; import com.google.common.css.compiler.ast.CssBooleanExpressionNode; import com.google.common.css.compiler.ast.CssCompilerPass; import com.google.common.css.compiler.ast.CssConditionalBlockNode; @@ -108,7 +107,7 @@ public void leaveUnknownAtRule(CssUnknownAtRuleNode node) { @Override public boolean enterRuleset(CssRulesetNode node) { activeBlockNode = null; - return false; + return true; } @Override @@ -119,9 +118,7 @@ public boolean enterDefinition(CssDefinitionNode node) { private CssConditionalRuleNode createConditionalRuleNode( CssUnknownAtRuleNode node, String name) { - Preconditions.checkState(node.getBlock() == null - || node.getBlock() instanceof CssBlockNode); - CssBlockNode block = (CssBlockNode) node.getBlock(); + CssAbstractBlockNode block = node.getBlock(); if (block == null) { errorManager.report(new GssError("@" + name + " without block", node.getSourceCodeLocation())); diff --git a/src/com/google/common/css/compiler/passes/PassRunner.java b/src/com/google/common/css/compiler/passes/PassRunner.java index 6dd0836d..4c693d6a 100644 --- a/src/com/google/common/css/compiler/passes/PassRunner.java +++ b/src/com/google/common/css/compiler/passes/PassRunner.java @@ -96,6 +96,11 @@ public void runPasses(CssTree cssTree) { new ProcessRefiners(cssTree.getMutatingVisitController(), errorManager, job.simplifyCss).runPass(); + // Eliminate conditional nodes. + new EliminateConditionalNodes( + cssTree.getMutatingVisitController(), + ImmutableSet.copyOf(job.trueConditionNames)).runPass(); + // Collect mixin definitions and replace mixins CollectMixinDefinitions collectMixinDefinitions = new CollectMixinDefinitions(cssTree.getMutatingVisitController(), @@ -106,10 +111,6 @@ public void runPasses(CssTree cssTree) { new ProcessComponents(cssTree.getMutatingVisitController(), errorManager).runPass(); - // Eliminate conditional nodes. - new EliminateConditionalNodes( - cssTree.getMutatingVisitController(), - ImmutableSet.copyOf(job.trueConditionNames)).runPass(); // Collect constant definitions. CollectConstantDefinitions collectConstantDefinitionsPass = new CollectConstantDefinitions(cssTree); diff --git a/tests/com/google/common/css/compiler/passes/CreateConditionalNodesTest.java b/tests/com/google/common/css/compiler/passes/CreateConditionalNodesTest.java index 2fccef72..51417747 100644 --- a/tests/com/google/common/css/compiler/passes/CreateConditionalNodesTest.java +++ b/tests/com/google/common/css/compiler/passes/CreateConditionalNodesTest.java @@ -18,7 +18,9 @@ import com.google.common.css.compiler.ast.CssConditionalBlockNode; import com.google.common.css.compiler.ast.CssConditionalRuleNode; +import com.google.common.css.compiler.ast.CssDeclarationBlockNode; import com.google.common.css.compiler.ast.CssNode; +import com.google.common.css.compiler.ast.CssRulesetNode; import com.google.common.css.compiler.ast.testing.NewFunctionalTestBase; /** @@ -72,6 +74,26 @@ public void testCreateNestedConditionalBlockNode() throws Exception { assertEquals("[[d]{[e:[f]]}]", elseCondRuleIf.getBlock().toString()); } + public void testCreateConditionalBlockNodeInRuleset() throws Exception { + parseAndRun("a {@if X {b: c} @else {d: e} }"); + assertTrue(getFirstActualNode() instanceof CssRulesetNode); + CssRulesetNode ruleset = (CssRulesetNode) getFirstActualNode(); + assertEquals("[a]{[[@if[X]{[b:[c]]}, @else[]{[d:[e]]}]]}", ruleset.toString()); + CssDeclarationBlockNode declarationBlock = ruleset.getDeclarations(); + assertEquals(1, declarationBlock.getChildren().size()); + assertTrue(declarationBlock.getChildAt(0) instanceof CssConditionalBlockNode); + CssConditionalBlockNode condBlock = (CssConditionalBlockNode) declarationBlock.getChildAt(0); + assertEquals(2, condBlock.getChildren().size()); + CssConditionalRuleNode condRuleIf = condBlock.getChildren().get(0); + CssConditionalRuleNode condRuleElse = condBlock.getChildren().get(1); + assertEquals("if", condRuleIf.getName().getValue()); + assertEquals(1, condRuleIf.getParametersCount()); + assertEquals("[b:[c]]", condRuleIf.getBlock().toString()); + assertEquals("else", condRuleElse.getName().getValue()); + assertEquals(0, condRuleElse.getParametersCount()); + assertEquals("[d:[e]]", condRuleElse.getBlock().toString()); + } + public void testIfWithoutBlockError() throws Exception { parseAndRun("@if (X) ;", "@if without block"); }