From b6feca3324b400e7abf870515ec8213715187c71 Mon Sep 17 00:00:00 2001 From: Haijian Wang Date: Tue, 26 Feb 2013 13:32:28 +0200 Subject: [PATCH] Fixed several problems related to @extend directive (Ticket #10976) Change-Id: I5e409856601aa514965319453c11946028b08dda --- .../vaadin/sass/internal/ScssStylesheet.java | 2 + .../vaadin/sass/internal/util/StringUtil.java | 78 ++++++++++++++++--- .../internal/visitor/ExtendNodeHandler.java | 57 +++++++++++--- .../automatic/css/extend-in-nested-block.css | 7 ++ .../extend-selector-in-different-levels.css | 15 ++++ ...non-exist-selector-with-same-beginning.css | 7 ++ .../automatic/css/extending-same-selector.css | 7 ++ ...extending-selector-with-same-beginning.css | 7 ++ .../scss/extend-in-nested-block.scss | 11 +++ .../extend-selector-in-different-levels.scss | 26 +++++++ ...on-exist-selector-with-same-beginning.scss | 8 ++ .../scss/extending-same-selector.scss | 8 ++ ...xtending-selector-with-same-beginning.scss | 8 ++ .../css/19-test_control_flow_if.css | 0 .../scss/19-test_control_flow_if.scss | 0 .../sass/internal/util/StringUtilTest.java | 58 ++++++++++++++ 16 files changed, 277 insertions(+), 22 deletions(-) create mode 100644 theme-compiler/tests/resources/automatic/css/extend-in-nested-block.css create mode 100644 theme-compiler/tests/resources/automatic/css/extend-selector-in-different-levels.css create mode 100644 theme-compiler/tests/resources/automatic/css/extending-non-exist-selector-with-same-beginning.css create mode 100644 theme-compiler/tests/resources/automatic/css/extending-same-selector.css create mode 100644 theme-compiler/tests/resources/automatic/css/extending-selector-with-same-beginning.css create mode 100644 theme-compiler/tests/resources/automatic/scss/extend-in-nested-block.scss create mode 100644 theme-compiler/tests/resources/automatic/scss/extend-selector-in-different-levels.scss create mode 100644 theme-compiler/tests/resources/automatic/scss/extending-non-exist-selector-with-same-beginning.scss create mode 100644 theme-compiler/tests/resources/automatic/scss/extending-same-selector.scss create mode 100644 theme-compiler/tests/resources/automatic/scss/extending-selector-with-same-beginning.scss rename theme-compiler/tests/resources/{sasslangbroken => sasslang}/css/19-test_control_flow_if.css (100%) rename theme-compiler/tests/resources/{sasslangbroken => sasslang}/scss/19-test_control_flow_if.scss (100%) diff --git a/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java b/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java index 64279ad1e7..688d569dff 100644 --- a/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java +++ b/theme-compiler/src/com/vaadin/sass/internal/ScssStylesheet.java @@ -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); diff --git a/theme-compiler/src/com/vaadin/sass/internal/util/StringUtil.java b/theme-compiler/src/com/vaadin/sass/internal/util/StringUtil.java index cf227fe3a3..b20e8bab61 100644 --- a/theme-compiler/src/com/vaadin/sass/internal/util/StringUtil.java +++ b/theme-compiler/src/com/vaadin/sass/internal/util/StringUtil.java @@ -17,8 +17,10 @@ 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("(? subStrings = Arrays.asList(motherString.split(Pattern + .quote(splitter))); + LinkedHashSet uniqueSubStrings = new LinkedHashSet( + 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(); + } } diff --git a/theme-compiler/src/com/vaadin/sass/internal/visitor/ExtendNodeHandler.java b/theme-compiler/src/com/vaadin/sass/internal/visitor/ExtendNodeHandler.java index f7917fff6e..e4a69ea5f3 100644 --- a/theme-compiler/src/com/vaadin/sass/internal/visitor/ExtendNodeHandler.java +++ b/theme-compiler/src/com/vaadin/sass/internal/visitor/ExtendNodeHandler.java @@ -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>> extendsMap = new HashMap>>(); @@ -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>> entry : extendsMap .entrySet()) { - if (selectorString.contains(entry.getKey())) { + if (StringUtil.containsSubString(selectorString, + entry.getKey())) { for (ArrayList sList : entry.getValue()) { ArrayList clone = (ArrayList) sList .clone(); @@ -71,22 +79,36 @@ public class ExtendNodeHandler { if (extendsMap.get(extendedString) == null) { extendsMap.put(extendedString, new ArrayList>()); } - 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 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 extendingSelectors, + String extendedSelector) { + if (extendingSelectors != null) { + for (String extendingSelector : extendingSelectors) { + if (extendedSelector == null) { + blockNode.getSelectorList().add(extendingSelector); } else { ArrayList newTags = new ArrayList(); - 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 index 0000000000..29f1550dd7 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/extend-in-nested-block.css @@ -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 index 0000000000..4de05d8d82 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/extend-selector-in-different-levels.css @@ -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 index 0000000000..d138a79e4a --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/extending-non-exist-selector-with-same-beginning.css @@ -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 index 0000000000..1a85c0c23e --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/extending-same-selector.css @@ -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 index 0000000000..097d7a8655 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/css/extending-selector-with-same-beginning.css @@ -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 index 0000000000..d62ead937e --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/extend-in-nested-block.scss @@ -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 index 0000000000..977ead8d62 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/extend-selector-in-different-levels.scss @@ -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 index 0000000000..538f17da1d --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/extending-non-exist-selector-with-same-beginning.scss @@ -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 index 0000000000..fbfaed9b20 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/extending-same-selector.scss @@ -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 index 0000000000..c7a9e5e921 --- /dev/null +++ b/theme-compiler/tests/resources/automatic/scss/extending-selector-with-same-beginning.scss @@ -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/sasslangbroken/css/19-test_control_flow_if.css b/theme-compiler/tests/resources/sasslang/css/19-test_control_flow_if.css similarity index 100% rename from theme-compiler/tests/resources/sasslangbroken/css/19-test_control_flow_if.css rename to theme-compiler/tests/resources/sasslang/css/19-test_control_flow_if.css diff --git a/theme-compiler/tests/resources/sasslangbroken/scss/19-test_control_flow_if.scss b/theme-compiler/tests/resources/sasslang/scss/19-test_control_flow_if.scss similarity index 100% rename from theme-compiler/tests/resources/sasslangbroken/scss/19-test_control_flow_if.scss rename to theme-compiler/tests/resources/sasslang/scss/19-test_control_flow_if.scss diff --git a/theme-compiler/tests/src/com/vaadin/sass/internal/util/StringUtilTest.java b/theme-compiler/tests/src/com/vaadin/sass/internal/util/StringUtilTest.java index b05b0e9dcf..84d189d8ba 100644 --- a/theme-compiler/tests/src/com/vaadin/sass/internal/util/StringUtilTest.java +++ b/theme-compiler/tests/src/com/vaadin/sass/internal/util/StringUtilTest.java @@ -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", ".")); + } } -- 2.39.5