From 9eba3dd4edff07622cf1bd75cb5e1e97300eb12d Mon Sep 17 00:00:00 2001 From: Bas Leijdekkers Date: Tue, 31 Jan 2017 19:24:42 +0100 Subject: [PATCH] RegExp: use code point instead of char for character value --- .../intellij/lang/regexp/psi/RegExpChar.java | 9 +-- .../lang/regexp/psi/impl/RegExpCharImpl.java | 31 ++++----- .../regexp/validation/RegExpAnnotator.java | 65 ++++++++++--------- .../RemoveRedundantEscapeAction.java | 23 ++++--- .../codeInsight/RegExpHighlightingTest.java | 6 ++ .../plugins/intelliLang/util/RegExpUtil.java | 2 +- 6 files changed, 71 insertions(+), 65 deletions(-) diff --git a/RegExpSupport/src/org/intellij/lang/regexp/psi/RegExpChar.java b/RegExpSupport/src/org/intellij/lang/regexp/psi/RegExpChar.java index 592f1124f7f9..096aeef65a53 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/psi/RegExpChar.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/psi/RegExpChar.java @@ -16,7 +16,6 @@ package org.intellij.lang.regexp.psi; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; /** * Represents a simple or escaped character @@ -39,10 +38,6 @@ public interface RegExpChar extends RegExpAtom, RegExpClassElement, RegExpCharRa @NotNull Type getType(); - /** - * Returns possibly unescaped character-value. - * Null if escape sequence is invalid. - */ - @Nullable - Character getValue(); + /** Returns unescaped character code point value, -1 if escape sequence is invalid. */ + int getValue(); } diff --git a/RegExpSupport/src/org/intellij/lang/regexp/psi/impl/RegExpCharImpl.java b/RegExpSupport/src/org/intellij/lang/regexp/psi/impl/RegExpCharImpl.java index 570a8ba56db0..8ab33635e926 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/psi/impl/RegExpCharImpl.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/psi/impl/RegExpCharImpl.java @@ -20,12 +20,11 @@ import com.intellij.psi.StringEscapesTokenTypes; import com.intellij.psi.TokenType; import com.intellij.psi.tree.IElementType; import com.intellij.psi.tree.TokenSet; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; - import org.intellij.lang.regexp.RegExpTT; +import org.intellij.lang.regexp.UnicodeCharacterNames; import org.intellij.lang.regexp.psi.RegExpChar; import org.intellij.lang.regexp.psi.RegExpElementVisitor; +import org.jetbrains.annotations.NotNull; public class RegExpCharImpl extends RegExpElementImpl implements RegExpChar { private static final TokenSet OCT_CHARS = TokenSet.create(RegExpTT.OCT_CHAR, RegExpTT.BAD_OCT_VALUE); @@ -56,15 +55,13 @@ public class RegExpCharImpl extends RegExpElementImpl implements RegExpChar { } @Override - @Nullable - public Character getValue() { + public int getValue() { final String s = getUnescapedText(); if (s.equals("\\") && getType() == Type.CHAR) return '\\'; return unescapeChar(s); } - @Nullable - private static Character unescapeChar(String s) { + private static int unescapeChar(String s) { final int length = s.length(); assert length > 0; @@ -96,20 +93,20 @@ public class RegExpCharImpl extends RegExpElementImpl implements RegExpChar { case 'c': return (char)(ch ^ 64); case 'x': - if (length <= idx + 1) return null; + if (length <= idx + 1) return -1; if (s.charAt(idx + 1) == '{') { final char c = s.charAt(length - 1); - return (c != '}') ? null : parseNumber(s, idx + 2, 16); + return (c != '}') ? -1 : parseNumber(s, idx + 2, 16); } if (length == 3) { return parseNumber(s, idx + 1, 16); } - return length == 4 ? parseNumber(s, idx + 1, 16) : null; + return length == 4 ? parseNumber(s, idx + 1, 16) : -1; case 'u': - if (length <= idx + 1) return null; + if (length <= idx + 1) return -1; if (length > idx + 1 && s.charAt(idx + 1) == '{') { final char c = s.charAt(length - 1); - return (c != '}') ? null : parseNumber(s, idx + 2, 16); + return (c != '}') ? -1 : parseNumber(s, idx + 2, 16); } if (length != 6) { return ch; @@ -130,10 +127,10 @@ public class RegExpCharImpl extends RegExpElementImpl implements RegExpChar { } } - return null; + return -1; } - private static Character parseNumber(String s, int offset, int radix) { + private static int parseNumber(String s, int offset, int radix) { int sum = 0; int i = offset; for (; i < s.length(); i++) { @@ -143,11 +140,11 @@ public class RegExpCharImpl extends RegExpElementImpl implements RegExpChar { } sum = sum * radix + digit; if (sum > Character.MAX_CODE_POINT) { - return null; + return -1; } } - if (i - offset <= 0) return null; // no digits found - return (char)sum; + if (i - offset <= 0) return -1; // no digits found + return sum; } @Override diff --git a/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java b/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java index 2d901806f688..e5bb1fbc1240 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/validation/RegExpAnnotator.java @@ -41,7 +41,6 @@ import java.util.Set; public final class RegExpAnnotator extends RegExpElementVisitor implements Annotator { private static final Set POSIX_CHARACTER_CLASSES = ContainerUtil.newHashSet( "alnum", "alpha", "ascii", "blank", "cntrl", "digit", "graph", "lower", "print", "punct", "space", "upper", "word", "xdigit"); - private static final String ILLEGAL_CHARACTER_RANGE_TO_FROM = "Illegal character range (to < from)"; private AnnotationHolder myHolder; private final RegExpLanguageHosts myLanguageHosts; @@ -87,17 +86,7 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot final RegExpCharRange.Endpoint from = range.getFrom(); final RegExpCharRange.Endpoint to = range.getTo(); if (from instanceof RegExpChar && to instanceof RegExpChar) { - final Character t = ((RegExpChar)to).getValue(); - final Character f = ((RegExpChar)from).getValue(); - if (t != null && f != null) { - if (t < f) { - if (handleSurrogates(range, f, t)) return; - myHolder.createErrorAnnotation(range, ILLEGAL_CHARACTER_RANGE_TO_FROM); - } - else if (t == f) { - myHolder.createWarningAnnotation(range, "Redundant character range"); - } - } + checkRange(range, ((RegExpChar)from).getValue(), ((RegExpChar)to).getValue()); } else if (to instanceof RegExpSimpleClass) { myHolder.createErrorAnnotation(to, "Character class not allowed inside character range"); @@ -107,27 +96,39 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot } } - private boolean handleSurrogates(RegExpCharRange range, Character f, Character t) { + private void checkRange(RegExpCharRange range, int fromCodePoint, int toCodePoint) { + if (fromCodePoint == -1 || toCodePoint == -1) { + return; + } + int errorStart = range.getTextOffset(); + int errorEnd = errorStart + range.getTextLength(); // \ud800\udc00-\udbff\udfff - final PsiElement prevSibling = range.getPrevSibling(); - final PsiElement nextSibling = range.getNextSibling(); - - if (prevSibling instanceof RegExpChar && nextSibling instanceof RegExpChar) { - final Character prevSiblingValue = ((RegExpChar)prevSibling).getValue(); - final Character nextSiblingValue = ((RegExpChar)nextSibling).getValue(); - - if (prevSiblingValue != null && nextSiblingValue != null && - Character.isSurrogatePair(prevSiblingValue, f) && Character.isSurrogatePair(t, nextSiblingValue)) { - if (Character.toCodePoint(prevSiblingValue, f) > Character.toCodePoint(t, nextSiblingValue)) { - final TextRange prevSiblingRange = prevSibling.getTextRange(); - final TextRange nextSiblingRange = nextSibling.getTextRange(); - final TextRange errorRange = new TextRange(prevSiblingRange.getStartOffset(), nextSiblingRange.getEndOffset()); - myHolder.createErrorAnnotation(errorRange, ILLEGAL_CHARACTER_RANGE_TO_FROM); + if (!Character.isSupplementaryCodePoint(fromCodePoint) && Character.isLowSurrogate((char)fromCodePoint)) { + final PsiElement prevSibling = range.getPrevSibling(); + if (prevSibling instanceof RegExpChar) { + final int prevSiblingValue = ((RegExpChar)prevSibling).getValue(); + if (!Character.isSupplementaryCodePoint(prevSiblingValue) && Character.isHighSurrogate((char)prevSiblingValue)) { + fromCodePoint = Character.toCodePoint((char)prevSiblingValue, (char)fromCodePoint); + errorStart -= prevSibling.getTextLength(); } - return true; } } - return false; + if (!Character.isSupplementaryCodePoint(toCodePoint) && Character.isHighSurrogate((char)toCodePoint)) { + final PsiElement nextSibling = range.getNextSibling(); + if (nextSibling instanceof RegExpChar) { + final int nextSiblingValue = ((RegExpChar)nextSibling).getValue(); + if (!Character.isSupplementaryCodePoint(nextSiblingValue) && Character.isLowSurrogate((char)nextSiblingValue)) { + toCodePoint = Character.toCodePoint((char)toCodePoint, (char)nextSiblingValue); + errorEnd += nextSibling.getTextLength(); + } + } + } + if (toCodePoint < fromCodePoint) { + myHolder.createErrorAnnotation(new TextRange(errorStart, errorEnd), "Illegal character range (to < from)"); + } + else if (toCodePoint == fromCodePoint) { + myHolder.createWarningAnnotation(new TextRange(errorStart, errorEnd), "Redundant character range"); + } } @Override @@ -154,8 +155,8 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot 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)) { + final int value = regExpChar.getValue(); + if (value != -1 && !seen.add(value)) { myHolder.createWarningAnnotation(regExpChar, "Duplicate character '" + regExpChar.getText() + "' inside character class"); } } @@ -208,7 +209,7 @@ public final class RegExpAnnotator extends RegExpElementVisitor implements Annot } final RegExpChar.Type charType = ch.getType(); if (charType == RegExpChar.Type.HEX || charType == RegExpChar.Type.UNICODE) { - if (ch.getValue() == null) { + if (ch.getValue() == -1) { myHolder.createErrorAnnotation(ch, "Illegal unicode escape sequence"); return; } diff --git a/RegExpSupport/src/org/intellij/lang/regexp/validation/RemoveRedundantEscapeAction.java b/RegExpSupport/src/org/intellij/lang/regexp/validation/RemoveRedundantEscapeAction.java index 4b399af148c2..7a8536988a71 100644 --- a/RegExpSupport/src/org/intellij/lang/regexp/validation/RemoveRedundantEscapeAction.java +++ b/RegExpSupport/src/org/intellij/lang/regexp/validation/RemoveRedundantEscapeAction.java @@ -56,8 +56,8 @@ class RemoveRedundantEscapeAction implements IntentionAction { @Override public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException { - final Character v = myChar.getValue(); - assert v != null; + final int v = myChar.getValue(); + assert v != -1; final ASTNode node = myChar.getNode().getFirstChildNode(); final ASTNode parent = node.getTreeParent(); @@ -66,13 +66,20 @@ class RemoveRedundantEscapeAction implements IntentionAction { } @NotNull - private String replacement(@NotNull Character v) { + private String replacement(int codePoint) { final PsiElement context = myChar.getContainingFile().getContext(); - return RegExpElementImpl.isLiteralExpression(context) ? - StringUtil.escapeStringCharacters(v.toString()) : - context instanceof XmlElement ? - XmlStringUtil.escapeString(v.toString()) : - v.toString(); + String s = Character.isSupplementaryCodePoint(codePoint) + ? Character.toString(Character.highSurrogate(codePoint)) + Character.toString(Character.lowSurrogate(codePoint)) + : Character.toString((char)codePoint); + if (RegExpElementImpl.isLiteralExpression(context)) { + return StringUtil.escapeStringCharacters(s); + } + else if (context instanceof XmlElement) { + return XmlStringUtil.escapeString(s); + } + else { + return s; + } } @Override diff --git a/java/java-tests/testSrc/com/intellij/codeInsight/RegExpHighlightingTest.java b/java/java-tests/testSrc/com/intellij/codeInsight/RegExpHighlightingTest.java index 69318f732372..bf02dcc08c39 100644 --- a/java/java-tests/testSrc/com/intellij/codeInsight/RegExpHighlightingTest.java +++ b/java/java-tests/testSrc/com/intellij/codeInsight/RegExpHighlightingTest.java @@ -130,6 +130,12 @@ public class RegExpHighlightingTest extends LightCodeInsightFixtureTestCase { doTest("[z-a]"); } + public void testLegalCharacterRange() { + // Cyrillic Capital Letter Zemlya - Unicode Han Character 'to peel, pare' (Unicode Supplementary Character) + // without code point support 0x20731 wraps to 0x731 which would produce a "Illegal character range (to < from)" error + doTest("[\\x{A640}-\\x{20731}]"); + } + public void testQuoted() { doTest("[\\Qabc?*+.))]][]\\E]"); } diff --git a/plugins/IntelliLang/src/org/intellij/plugins/intelliLang/util/RegExpUtil.java b/plugins/IntelliLang/src/org/intellij/plugins/intelliLang/util/RegExpUtil.java index 2c0febeb0df3..114281f7a178 100644 --- a/plugins/IntelliLang/src/org/intellij/plugins/intelliLang/util/RegExpUtil.java +++ b/plugins/IntelliLang/src/org/intellij/plugins/intelliLang/util/RegExpUtil.java @@ -54,7 +54,7 @@ public class RegExpUtil { private static boolean analyzeBranch(RegExpBranch branch) { final RegExpAtom[] atoms = branch.getAtoms(); for (RegExpAtom atom : atoms) { - if (!(atom instanceof RegExpChar) || ((RegExpChar)atom).getValue() == null) { + if (!(atom instanceof RegExpChar) || ((RegExpChar)atom).getValue() == -1) { return false; } else if (((RegExpChar)atom).getType() != RegExpChar.Type.CHAR) {