]> source.dussan.org Git - vaadin-framework.git/commitdiff
Fixed several problems related to @extend directive (Ticket #10976)
authorHaijian Wang <haijian@vaadin.com>
Tue, 26 Feb 2013 11:32:28 +0000 (13:32 +0200)
committerVaadin Code Review <review@vaadin.com>
Tue, 26 Feb 2013 14:37:42 +0000 (14:37 +0000)
Change-Id: I5e409856601aa514965319453c11946028b08dda

18 files changed:
theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java
theme-compiler/src/com/vaadin/sass/internal/util/StringUtil.java
theme-compiler/src/com/vaadin/sass/internal/visitor/ExtendNodeHandler.java
theme-compiler/tests/resources/automatic/css/extend-in-nested-block.css [new file with mode: 0644]
theme-compiler/tests/resources/automatic/css/extend-selector-in-different-levels.css [new file with mode: 0644]
theme-compiler/tests/resources/automatic/css/extending-non-exist-selector-with-same-beginning.css [new file with mode: 0644]
theme-compiler/tests/resources/automatic/css/extending-same-selector.css [new file with mode: 0644]
theme-compiler/tests/resources/automatic/css/extending-selector-with-same-beginning.css [new file with mode: 0644]
theme-compiler/tests/resources/automatic/scss/extend-in-nested-block.scss [new file with mode: 0644]
theme-compiler/tests/resources/automatic/scss/extend-selector-in-different-levels.scss [new file with mode: 0644]
theme-compiler/tests/resources/automatic/scss/extending-non-exist-selector-with-same-beginning.scss [new file with mode: 0644]
theme-compiler/tests/resources/automatic/scss/extending-same-selector.scss [new file with mode: 0644]
theme-compiler/tests/resources/automatic/scss/extending-selector-with-same-beginning.scss [new file with mode: 0644]
theme-compiler/tests/resources/sasslang/css/19-test_control_flow_if.css [new file with mode: 0644]
theme-compiler/tests/resources/sasslang/scss/19-test_control_flow_if.scss [new file with mode: 0644]
theme-compiler/tests/resources/sasslangbroken/css/19-test_control_flow_if.css [deleted file]
theme-compiler/tests/resources/sasslangbroken/scss/19-test_control_flow_if.scss [deleted file]
theme-compiler/tests/src/com/vaadin/sass/internal/util/StringUtilTest.java

index 64279ad1e70dd8803c5b39c309aea883c14164b9..688d569dffc11edc41f687ab49220fb57640771c 100644 (file)
@@ -42,6 +42,7 @@ import com.vaadin.sass.internal.tree.MixinDefNode;
 import com.vaadin.sass.internal.tree.Node;
 import com.vaadin.sass.internal.tree.VariableNode;
 import com.vaadin.sass.internal.tree.controldirective.IfElseDefNode;
+import com.vaadin.sass.internal.visitor.ExtendNodeHandler;
 import com.vaadin.sass.internal.visitor.ImportNodeHandler;
 
 public class ScssStylesheet extends Node {
@@ -172,6 +173,7 @@ public class ScssStylesheet extends Node {
         variables.clear();
         ifElseDefNodes.clear();
         lastNodeAdded.clear();
+        ExtendNodeHandler.clear();
         importOtherFiles(this);
         populateDefinitions(this);
         traverse(this);
index cf227fe3a316f1364b06a5d793060c4b07b4fac4..b20e8bab61b4f50fd65eeb3377ae4e746fb973e5 100644 (file)
 package com.vaadin.sass.internal.util;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.regex.Matcher;
@@ -145,14 +147,7 @@ public class StringUtil {
      * @return true if the text contains the SCSS variable, false if not
      */
     public static boolean containsVariable(String text, String varName) {
-        StringBuilder builder = new StringBuilder();
-        // (?![\\w-]) means lookahead, the next one shouldn't be a word
-        // character nor a dash.
-        builder.append("\\$").append(Pattern.quote(varName))
-                .append("(?![\\w-])");
-        Pattern pattern = Pattern.compile(builder.toString());
-        Matcher matcher = pattern.matcher(text);
-        return matcher.find();
+        return containsSubString(text, "$" + varName);
     }
 
     /**
@@ -162,18 +157,81 @@ public class StringUtil {
      * @param text
      *            text which contains the SCSS variable
      * @param varName
-     *            SCSS variable name
+     *            SCSS variable name (Without '$' sign)
      * @param value
      *            the value of the SCSS variable
      * @return the String after replacing
      */
     public static String replaceVariable(String text, String varName,
             String value) {
+        return replaceSubString(text, "$" + varName, value);
+    }
+
+    /**
+     * Check if a String contains a sub string, using whole word match.
+     * 
+     * @param text
+     *            text to be checked
+     * @Param sub Sub String to be checked.
+     * @return true if the text contains the sub string, false if not
+     */
+    public static boolean containsSubString(String text, String sub) {
         StringBuilder builder = new StringBuilder();
         // (?![\\w-]) means lookahead, the next one shouldn't be a word
         // character nor a dash.
-        builder.append("\\$").append(Pattern.quote(varName))
+        builder.append("(?<![\\w-])").append(Pattern.quote(sub))
+                .append("(?![\\w-])");
+        Pattern pattern = Pattern.compile(builder.toString());
+        Matcher matcher = pattern.matcher(text);
+        return matcher.find();
+    }
+
+    /**
+     * Replace the sub string in a String to a value, using whole word match.
+     * 
+     * @param text
+     *            text which contains the sub string
+     * @param sub
+     *            the sub string
+     * @param value
+     *            the new value
+     * @return the String after replacing
+     */
+    public static String replaceSubString(String text, String sub, String value) {
+        StringBuilder builder = new StringBuilder();
+        // (?![\\w-]) means lookahead, the next one shouldn't be a word
+        // character nor a dash.
+        builder.append("(?<![\\w-])").append(Pattern.quote(sub))
                 .append("(?![\\w-])");
         return text.replaceAll(builder.toString(), value);
     }
+
+    /**
+     * Remove duplicated sub string in a String given a splitter. Can be used to
+     * removed duplicated selectors, e.g., in ".error.error", one duplicated
+     * ".error" can be removed.
+     * 
+     * @param motherString
+     *            string which may contains duplicated sub strings
+     * @param splitter
+     *            the splitter splits the mother string to sub strings
+     * @return the mother string with duplicated sub strings removed
+     */
+    public static String removeDuplicatedSubString(String motherString,
+            String splitter) {
+        List<String> subStrings = Arrays.asList(motherString.split(Pattern
+                .quote(splitter)));
+        LinkedHashSet<String> uniqueSubStrings = new LinkedHashSet<String>(
+                subStrings);
+        StringBuilder builder = new StringBuilder();
+        int count = 0;
+        for (String uniqueSubString : uniqueSubStrings) {
+            count++;
+            builder.append(uniqueSubString);
+            if (count < uniqueSubStrings.size()) {
+                builder.append(splitter);
+            }
+        }
+        return builder.toString();
+    }
 }
index f7917fff6e9fbf13a69510dabb3331c25f978296..e4a69ea5f3696f3c0f32bf0da9abcc1b50434f09 100644 (file)
@@ -26,6 +26,7 @@ import com.vaadin.sass.internal.ScssStylesheet;
 import com.vaadin.sass.internal.tree.BlockNode;
 import com.vaadin.sass.internal.tree.ExtendNode;
 import com.vaadin.sass.internal.tree.Node;
+import com.vaadin.sass.internal.util.StringUtil;
 
 public class ExtendNodeHandler {
     private static Map<String, List<ArrayList<String>>> extendsMap = new HashMap<String, List<ArrayList<String>>>();
@@ -35,6 +36,12 @@ public class ExtendNodeHandler {
         modifyTree(ScssStylesheet.get());
     }
 
+    public static void clear() {
+        if (extendsMap != null) {
+            extendsMap.clear();
+        }
+    }
+
     private static void modifyTree(Node node) throws Exception {
         for (Node child : node.getChildren()) {
             if (child instanceof BlockNode) {
@@ -51,7 +58,8 @@ public class ExtendNodeHandler {
                 } else {
                     for (Entry<String, List<ArrayList<String>>> entry : extendsMap
                             .entrySet()) {
-                        if (selectorString.contains(entry.getKey())) {
+                        if (StringUtil.containsSubString(selectorString,
+                                entry.getKey())) {
                             for (ArrayList<String> sList : entry.getValue()) {
                                 ArrayList<String> clone = (ArrayList<String>) sList
                                         .clone();
@@ -71,22 +79,36 @@ public class ExtendNodeHandler {
         if (extendsMap.get(extendedString) == null) {
             extendsMap.put(extendedString, new ArrayList<ArrayList<String>>());
         }
-        extendsMap.get(extendedString).add(
-                ((BlockNode) node.getParentNode()).getSelectorList());
+        // prevent a selector extends itself, e.g. .test{ @extend .test}
+        String parentSelectorString = ((BlockNode) node.getParentNode())
+                .getSelectors();
+        if (!parentSelectorString.equals(extendedString)) {
+            extendsMap.get(extendedString).add(
+                    ((BlockNode) node.getParentNode()).getSelectorList());
+        }
     }
 
     private static void addAdditionalSelectorListToBlockNode(
-            BlockNode blockNode, ArrayList<String> list, String selectorString) {
-        if (list != null) {
-            for (int i = 0; i < list.size(); i++) {
-                if (selectorString == null) {
-                    blockNode.getSelectorList().add(list.get(i));
+            BlockNode blockNode, ArrayList<String> extendingSelectors,
+            String extendedSelector) {
+        if (extendingSelectors != null) {
+            for (String extendingSelector : extendingSelectors) {
+                if (extendedSelector == null) {
+                    blockNode.getSelectorList().add(extendingSelector);
                 } else {
                     ArrayList<String> newTags = new ArrayList<String>();
-                    for (final String existing : blockNode.getSelectorList()) {
-                        if (existing.contains(selectorString)) {
-                            newTags.add(existing.replace(selectorString,
-                                    list.get(i)));
+                    for (final String selectorString : blockNode
+                            .getSelectorList()) {
+                        if (StringUtil.containsSubString(selectorString,
+                                extendedSelector)) {
+                            String newTag = generateExtendingSelectors(
+                                    selectorString, extendedSelector,
+                                    extendingSelector);
+                            // prevent adding duplicated selector list
+                            if (!blockNode.getSelectorList().contains(newTag)
+                                    && !newTags.contains(newTag)) {
+                                newTags.add(newTag);
+                            }
                         }
                     }
                     blockNode.getSelectorList().addAll(newTags);
@@ -94,4 +116,15 @@ public class ExtendNodeHandler {
             }
         }
     }
+
+    private static String generateExtendingSelectors(String selectorString,
+            String extendedSelector, String extendingSelector) {
+        String result = StringUtil.replaceSubString(selectorString,
+                extendedSelector, extendingSelector);
+        // remove duplicated class selectors.
+        if (result.startsWith(".")) {
+            result = StringUtil.removeDuplicatedSubString(result, ".");
+        }
+        return result;
+    }
 }
diff --git a/theme-compiler/tests/resources/automatic/css/extend-in-nested-block.css b/theme-compiler/tests/resources/automatic/css/extend-in-nested-block.css
new file mode 100644 (file)
index 0000000..29f1550
--- /dev/null
@@ -0,0 +1,7 @@
+.test .error, .test .seriousError {
+       border: 1px #f00;
+       background-color: #fdd; 
+}
+.test .seriousError {
+       border-width: 3px; 
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/css/extend-selector-in-different-levels.css b/theme-compiler/tests/resources/automatic/css/extend-selector-in-different-levels.css
new file mode 100644 (file)
index 0000000..4de05d8
--- /dev/null
@@ -0,0 +1,15 @@
+.test .middle .error, .test .middle .seriousError {
+       border: 1px #f00;
+       background-color: #fdd; 
+}
+.test .seriousError {
+       border-width: 3px; 
+}
+
+.test1 .error1, .test1 .middle1 .seriousError1 {
+       border: 1px #f00;
+       background-color: #fdd; 
+}
+.test1 .middle1 .seriousError1 {
+       border-width: 3px;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/css/extending-non-exist-selector-with-same-beginning.css b/theme-compiler/tests/resources/automatic/css/extending-non-exist-selector-with-same-beginning.css
new file mode 100644 (file)
index 0000000..d138a79
--- /dev/null
@@ -0,0 +1,7 @@
+.test1 {
+       color: blue;
+}
+
+.test2 {
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/css/extending-same-selector.css b/theme-compiler/tests/resources/automatic/css/extending-same-selector.css
new file mode 100644 (file)
index 0000000..1a85c0c
--- /dev/null
@@ -0,0 +1,7 @@
+.test {
+       color: blue;
+}
+
+.test {
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/css/extending-selector-with-same-beginning.css b/theme-compiler/tests/resources/automatic/css/extending-selector-with-same-beginning.css
new file mode 100644 (file)
index 0000000..097d7a8
--- /dev/null
@@ -0,0 +1,7 @@
+.test1, .test2 {
+       color: blue;
+}
+
+.test2 {
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/scss/extend-in-nested-block.scss b/theme-compiler/tests/resources/automatic/scss/extend-in-nested-block.scss
new file mode 100644 (file)
index 0000000..d62ead9
--- /dev/null
@@ -0,0 +1,11 @@
+.test{
+ .error {
+               border: 1px #f00;
+               background-color: #fdd;
+       }
+
+ .seriousError {
+               @extend .error;
+               border-width: 3px;
+       }
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/scss/extend-selector-in-different-levels.scss b/theme-compiler/tests/resources/automatic/scss/extend-selector-in-different-levels.scss
new file mode 100644 (file)
index 0000000..977ead8
--- /dev/null
@@ -0,0 +1,26 @@
+.test{
+       .middle{
+               .error {
+                       border: 1px #f00;
+                       background-color: #fdd;
+               }
+       }
+
+       .seriousError {
+               @extend .error;
+               border-width: 3px;
+       }
+}
+
+.test1{
+       .error1 {
+               border: 1px #f00;
+               background-color: #fdd;
+       }
+       .middle1{
+               .seriousError1 {
+                       @extend .error1;
+                       border-width: 3px;
+               }
+       }
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/scss/extending-non-exist-selector-with-same-beginning.scss b/theme-compiler/tests/resources/automatic/scss/extending-non-exist-selector-with-same-beginning.scss
new file mode 100644 (file)
index 0000000..538f17d
--- /dev/null
@@ -0,0 +1,8 @@
+.test1 {
+       color: blue;
+}
+
+.test2 {
+       @extend .test;
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/scss/extending-same-selector.scss b/theme-compiler/tests/resources/automatic/scss/extending-same-selector.scss
new file mode 100644 (file)
index 0000000..fbfaed9
--- /dev/null
@@ -0,0 +1,8 @@
+.test {
+       color: blue;
+}
+
+.test {
+       @extend .test;
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/automatic/scss/extending-selector-with-same-beginning.scss b/theme-compiler/tests/resources/automatic/scss/extending-selector-with-same-beginning.scss
new file mode 100644 (file)
index 0000000..c7a9e5e
--- /dev/null
@@ -0,0 +1,8 @@
+.test1 {
+       color: blue;
+}
+
+.test2 {
+       @extend .test1;
+       background: red;
+}
\ No newline at end of file
diff --git a/theme-compiler/tests/resources/sasslang/css/19-test_control_flow_if.css b/theme-compiler/tests/resources/sasslang/css/19-test_control_flow_if.css
new file mode 100644 (file)
index 0000000..14a1c6e
--- /dev/null
@@ -0,0 +1,7 @@
+.true, .also-true {
+  color: green;
+}
+
+.false, .also-false {
+  color: red;
+}
diff --git a/theme-compiler/tests/resources/sasslang/scss/19-test_control_flow_if.scss b/theme-compiler/tests/resources/sasslang/scss/19-test_control_flow_if.scss
new file mode 100644 (file)
index 0000000..be53e52
--- /dev/null
@@ -0,0 +1,10 @@
+.true  { color: green; }
+.false { color: red;   }
+.also-true {
+  @if true { @extend .true;  }
+  @else    { @extend .false; }
+}
+.also-false {
+  @if false { @extend .true;  }
+  @else     { @extend .false; }
+}
diff --git a/theme-compiler/tests/resources/sasslangbroken/css/19-test_control_flow_if.css b/theme-compiler/tests/resources/sasslangbroken/css/19-test_control_flow_if.css
deleted file mode 100644 (file)
index 14a1c6e..0000000
+++ /dev/null
@@ -1,7 +0,0 @@
-.true, .also-true {
-  color: green;
-}
-
-.false, .also-false {
-  color: red;
-}
diff --git a/theme-compiler/tests/resources/sasslangbroken/scss/19-test_control_flow_if.scss b/theme-compiler/tests/resources/sasslangbroken/scss/19-test_control_flow_if.scss
deleted file mode 100644 (file)
index be53e52..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-.true  { color: green; }
-.false { color: red;   }
-.also-true {
-  @if true { @extend .true;  }
-  @else    { @extend .false; }
-}
-.also-false {
-  @if false { @extend .true;  }
-  @else     { @extend .false; }
-}
index b05b0e9dcf9c0a1996cbd9503c195680e388c42c..84d189d8baf475799994dfe311042aefec573708 100644 (file)
@@ -50,4 +50,62 @@ public class StringUtilTest {
         Assert.assertEquals(sentence,
                 StringUtil.replaceVariable(sentence, word, value));
     }
+
+    @Test
+    public void testContainsSubString() {
+        String sentence = "var1 var2";
+        String word = "var";
+        Assert.assertFalse(StringUtil.containsSubString(sentence, word));
+
+        word = "var1";
+        Assert.assertTrue(StringUtil.containsSubString(sentence, word));
+
+        String var2 = "var2";
+        Assert.assertTrue(StringUtil.containsSubString(sentence, var2));
+
+        Assert.assertTrue(StringUtil.containsSubString(".error.intrusion",
+                ".error"));
+
+        Assert.assertFalse(StringUtil.containsSubString(".foo", "oo"));
+    }
+
+    @Test
+    public void testContainsSubStringWithDash() {
+        String sentence = "var- var2";
+        String word = "var";
+        Assert.assertFalse(StringUtil.containsSubString(sentence, word));
+    }
+
+    @Test
+    public void testReplaceSubString() {
+        String sentence = "var1 var2";
+        String word = "var";
+        String value = "abc";
+
+        word = "var1";
+        Assert.assertEquals("abc var2",
+                StringUtil.replaceSubString(sentence, word, value));
+
+        String var2 = "var1 abc";
+        Assert.assertEquals(sentence,
+                StringUtil.replaceSubString(sentence, var2, value));
+
+        Assert.assertEquals(".foo",
+                StringUtil.replaceSubString(".foo", "oo", "aa"));
+    }
+
+    @Test
+    public void testReplaceSubStringWithDash() {
+        String sentence = "var- var2";
+        String word = "var";
+        String value = "abc";
+        Assert.assertEquals(sentence,
+                StringUtil.replaceSubString(sentence, word, value));
+    }
+
+    @Test
+    public void testRemoveDuplicatedClassSelector() {
+        Assert.assertEquals(".seriousError", StringUtil
+                .removeDuplicatedSubString(".seriousError.seriousError", "."));
+    }
 }