From 16ec0170f6fb7fec33b71ea361388468a19c7a4b Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Sun, 4 Dec 2016 15:56:09 +0100 Subject: [PATCH] regexp: fix a case of "good code is red" --- .../intellij/lang/regexp/RegExpParser.java | 16 +++++++---- .../org/intellij/lang/regexp/RegExpTT.java | 27 ++++++++----------- .../regexp/validation/RegExpAnnotator.java | 20 +++++++++----- RegExpSupport/testData/RETest.xml | 10 ++++--- RegExpSupport/testData/psi/Charclasses61.txt | 5 ++-- 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/RegExpSupport/src/org/intellij/lang/regexp/RegExpParser.java b/RegExpSupport/src/org/intellij/lang/regexp/RegExpParser.java index 06a6d3f3c77e..9c16d5334237 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/RegExpParser.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/RegExpParser.java @@ -217,7 +217,8 @@ public class RegExpParser implements PsiParser { // DEFLIST if (parseClassIntersection(builder)) { - while (RegExpTT.CHARACTERS2.contains(builder.getTokenType()) || + while (RegExpTT.CHARACTERS.contains(builder.getTokenType()) || + builder.getTokenType() == RegExpTT.CHAR_CLASS || builder.getTokenType() == RegExpTT.CLASS_BEGIN || builder.getTokenType() == RegExpTT.PROPERTY || builder.getTokenType() == RegExpTT.BRACKET_EXPRESSION_BEGIN) { @@ -257,9 +258,14 @@ public class RegExpParser implements PsiParser { else if (token == RegExpTT.BRACKET_EXPRESSION_BEGIN) { parseBracketExpression(builder); } - else if (RegExpTT.CHARACTERS2.contains(token)) { + else if (RegExpTT.CHARACTERS.contains(token)) { parseSimpleClassdef(builder); } + else if (token == RegExpTT.CHAR_CLASS) { + final PsiBuilder.Marker m = builder.mark(); + builder.advanceLexer(); + m.done(RegExpElementTypes.SIMPLE_CLASS); + } else if (token == RegExpTT.PROPERTY) { parseProperty(builder); } @@ -291,7 +297,7 @@ public class RegExpParser implements PsiParser { } private void parseSimpleClassdef(PsiBuilder builder) { - assert RegExpTT.CHARACTERS2.contains(builder.getTokenType()); + assert RegExpTT.CHARACTERS.contains(builder.getTokenType()); final PsiBuilder.Marker marker = builder.mark(); makeChar(builder); @@ -301,7 +307,7 @@ public class RegExpParser implements PsiParser { builder.advanceLexer(); final IElementType t = builder.getTokenType(); - if (RegExpTT.CHARACTERS2.contains(t)) { + if (RegExpTT.CHARACTERS.contains(t) || t == RegExpTT.CHAR_CLASS) { m.drop(); makeChar(builder); marker.done(RegExpElementTypes.CHAR_RANGE); @@ -449,7 +455,7 @@ public class RegExpParser implements PsiParser { marker.drop(); parseNamedCharacter(builder); } - else if (RegExpTT.SIMPLE_CLASSES.contains(type)) { + else if (type == RegExpTT.DOT || type == RegExpTT.CHAR_CLASS) { builder.advanceLexer(); marker.done(RegExpElementTypes.SIMPLE_CLASS); } diff --git a/RegExpSupport/src/org/intellij/lang/regexp/RegExpTT.java b/RegExpSupport/src/org/intellij/lang/regexp/RegExpTT.java index 6ad066f37b5d..2da30a8be0da 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/RegExpTT.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/RegExpTT.java @@ -135,22 +135,17 @@ public interface RegExpTT { IElementType RUBY_QUOTED_NAMED_GROUP_CALL = new RegExpElementType("RUBY_QUOTED_NAMED_GROUP_CALL"); TokenSet CHARACTERS = TokenSet.create(CHARACTER, - ESC_CTRL_CHARACTER, - ESC_CHARACTER, - CTRL_CHARACTER, - CTRL, - UNICODE_CHAR, - HEX_CHAR, BAD_HEX_VALUE, - OCT_CHAR, BAD_OCT_VALUE, - REDUNDANT_ESCAPE, - MINUS, - StringEscapesTokenTypes.INVALID_UNICODE_ESCAPE_TOKEN, - StringEscapesTokenTypes.INVALID_CHARACTER_ESCAPE_TOKEN); - - TokenSet SIMPLE_CLASSES = TokenSet.create(DOT, CHAR_CLASS); - - // caret is just a character in classes after the first position: [a^] matches "a" or "^" - TokenSet CHARACTERS2 = TokenSet.orSet(CHARACTERS, SIMPLE_CLASSES); + ESC_CTRL_CHARACTER, + ESC_CHARACTER, + CTRL_CHARACTER, + CTRL, + UNICODE_CHAR, + HEX_CHAR, BAD_HEX_VALUE, + OCT_CHAR, BAD_OCT_VALUE, + REDUNDANT_ESCAPE, + MINUS, + StringEscapesTokenTypes.INVALID_UNICODE_ESCAPE_TOKEN, + StringEscapesTokenTypes.INVALID_CHARACTER_ESCAPE_TOKEN); TokenSet QUANTIFIERS = TokenSet.create(QUEST, PLUS, STAR, LBRACE); diff --git a/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java b/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java index 3618b81df854..ee1a4ad01fda 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java @@ -84,9 +84,7 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot public void visitRegExpCharRange(RegExpCharRange range) { final RegExpCharRange.Endpoint from = range.getFrom(); final RegExpCharRange.Endpoint to = range.getTo(); - final boolean a = from instanceof RegExpChar; - final boolean b = to instanceof RegExpChar; - if (a && b) { + if (from instanceof RegExpChar && to instanceof RegExpChar) { final Character t = ((RegExpChar)to).getValue(); final Character f = ((RegExpChar)from).getValue(); if (t != null && f != null) { @@ -99,8 +97,8 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot } } } - else if (a != b) { - myHolder.createErrorAnnotation(range, "Character class (e.g. '\\\\w') may not be used inside character range"); + else if (to instanceof RegExpSimpleClass) { + myHolder.createErrorAnnotation(to, "Character class not allowed inside character range"); } else if (from.getText().equals(to.getText())) { myHolder.createWarningAnnotation(range, "Redundant character range"); @@ -151,12 +149,20 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot } } - private void checkForDuplicates(RegExpClassElement element, Set seen) { + private void checkForDuplicates(RegExpClassElement element, Set seen) { if (element instanceof RegExpChar) { final RegExpChar regExpChar = (RegExpChar)element; final Character value = regExpChar.getValue(); if (value != null && !seen.add(value)) { - myHolder.createWarningAnnotation(regExpChar, "Duplicate character '" + regExpChar.getText() + "' in character class"); + myHolder.createWarningAnnotation(regExpChar, "Duplicate character '" + regExpChar.getText() + "' inside character class"); + } + } + else if (element instanceof RegExpSimpleClass) { + final RegExpSimpleClass regExpSimpleClass = (RegExpSimpleClass)element; + final RegExpSimpleClass.Kind kind = regExpSimpleClass.getKind(); + if (!seen.add(kind)) { + myHolder.createWarningAnnotation(regExpSimpleClass, "Duplicate predefined character class '" + regExpSimpleClass.getText() + + "' inside character class"); } } else if (element instanceof RegExpClass) { diff --git a/RegExpSupport/testData/RETest.xml b/RegExpSupport/testData/RETest.xml index 75ca74a4696f..8069ee6fd0f8 100644 --- a/RegExpSupport/testData/RETest.xml +++ b/RegExpSupport/testData/RETest.xml @@ -37,7 +37,11 @@ - \w-\w]]]> + \w]]]> + OK + + + [\w-a] OK @@ -106,7 +110,7 @@ OK - )]][]\E]]]> + )]][]\E]]]> OK @@ -133,7 +137,7 @@ OK - it:]+]]> + it:]+]]> OK diff --git a/RegExpSupport/testData/psi/Charclasses61.txt b/RegExpSupport/testData/psi/Charclasses61.txt index 01e2ae8f4b48..13bfb2cb45dc 100644 --- a/RegExpSupport/testData/psi/Charclasses61.txt +++ b/RegExpSupport/testData/psi/Charclasses61.txt @@ -3,10 +3,11 @@ REGEXP_FILE RegExpBranchImpl: <[\w-\w]> RegExpClassImpl: <[\w-\w]> PsiElement(CLASS_BEGIN)('[') - RegExpCharRangeImpl: <\w-\w> + RegExpUnionImpl: <\w-\w> RegExpSimpleClassImpl: <\w> PsiElement(CHAR_CLASS)('\w') - PsiElement(MINUS)('-') + RegExpCharImpl: <-> + PsiElement(MINUS)('-') RegExpSimpleClassImpl: <\w> PsiElement(CHAR_CLASS)('\w') PsiElement(CLASS_END)(']') \ No newline at end of file