SSR: improve error checking for constraints strings

(quantifiers are now handled like in regular expressions)
2026-09-27 10:03:11 +07:00 · 2018-05-18 12:55:26 +02:00
parent 0b9fe52559
commit 90553cdc95
4 changed files with 130 additions and 93 deletions
@@ -4,9 +4,11 @@ package com.intellij.structuralsearch.impl.matcher.compiler;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.structuralsearch.*;
 import com.intellij.structuralsearch.plugin.ui.Configuration;
+import com.intellij.util.containers.ContainerUtil;
 import org.jetbrains.annotations.NonNls;
 import org.jetbrains.annotations.NotNull;

+import java.util.Set;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;

@@ -23,6 +25,8 @@ public class StringToConstraintsTransformer {
  @NonNls private static final String CONTAINS = "contains";
  @NonNls private static final String WITHIN = "within";

+  private static final Set<String> knownOptions = ContainerUtil.set(REF, REGEX, REGEXW, EXPRTYPE, FORMAL, SCRIPT, CONTAINS, WITHIN);
+
  @SuppressWarnings("AssignmentToForLoopParameter")
  public static void transformCriteria(String criteria, MatchOptions options) {
      final StringBuilder pattern = new StringBuilder();
@@ -103,8 +107,10 @@ public class StringToConstraintsTransformer {
              ++index;
            } else if (ch == '{') {
              ++index;
-              minOccurs = 0;
+              minOccurs = -1;
+              maxOccurs = -1;
              while (index < length && (ch = criteria.charAt(index)) >= '0' && ch <= '9') {
+                if (minOccurs < 0) minOccurs = 0;
                minOccurs = (minOccurs * 10) + (ch - '0');
                if (minOccurs < 0) throw new MalformedPatternException(SSRBundle.message("error.overflow"));
                ++index;
@@ -112,21 +118,26 @@ public class StringToConstraintsTransformer {

              if (ch==',') {
                ++index;
-                maxOccurs = 0;

                while (index < length && (ch = criteria.charAt(index)) >= '0' && ch <= '9') {
+                  if (maxOccurs < 0) maxOccurs = 0;
                  maxOccurs = (maxOccurs * 10) + (ch - '0');
                  if (maxOccurs < 0) throw new MalformedPatternException(SSRBundle.message("error.overflow"));
                  ++index;
                }
              } else {
-                maxOccurs = Integer.MAX_VALUE;
+                maxOccurs = -2;
              }

              if (ch != '}') {
-                if (maxOccurs == Integer.MAX_VALUE) throw new MalformedPatternException(SSRBundle.message("error.expected.brace1"));
+                if (minOccurs < 0 && maxOccurs < 0) throw new MalformedPatternException(SSRBundle.message("error.expected.digit"));
+                if (maxOccurs < 0) throw new MalformedPatternException(SSRBundle.message("error.expected.brace1"));
                else throw new MalformedPatternException(SSRBundle.message("error.expected.brace2"));
              }
+              if (minOccurs < 0 && maxOccurs < 0) throw new MalformedPatternException(SSRBundle.message("error.empty.quantifier"));
+              else if (minOccurs == -1) minOccurs = 0;
+              else if (maxOccurs == -1) maxOccurs = Integer.MAX_VALUE;
+              else if (maxOccurs == -2) maxOccurs = minOccurs;
              ++index;
            }

@@ -244,7 +255,7 @@ public class StringToConstraintsTransformer {
        }
      }
      if (quoted) throw new MalformedPatternException(SSRBundle.message("error.expected.value", "\""));
-      if (ch != ']') throw new MalformedPatternException(SSRBundle.message("error.expected.condition.or.bracket"));
+      if (ch != ']') throw new MalformedPatternException(SSRBundle.message("error.expected.value", "]"));
      parseCondition(constraint, criteria.substring(index, endIndex));
      return endIndex + 1;
    }
@@ -294,6 +305,9 @@ public class StringToConstraintsTransformer {
      else if (c == '(') {
        if (text.length() == 0) throw new MalformedPatternException(SSRBundle.message("error.expected.condition.name"));
        final String option = text.toString();
+        if (!knownOptions.contains(option)) {
+          throw new MalformedPatternException(SSRBundle.message("option.is.not.recognized.error.message", option));
+        }
        text.setLength(0);
        int spaces = 0; // balance spaces surrounding content between parentheses
        while (++i < length && condition.charAt(i) == ' ') spaces++;
@@ -315,6 +329,7 @@ public class StringToConstraintsTransformer {
          }
          text.append(c);
        }
+        if (text.length() == 0) throw new MalformedPatternException(SSRBundle.message("error.argument.expected", option));
        if (quoted) throw new MalformedPatternException(SSRBundle.message("error.expected.value", "\""));
        if (!closed) throw new MalformedPatternException(SSRBundle.message("error.expected.value",
                                                                           StringUtil.repeatSymbol(' ', spaces) + ")"));
@@ -349,19 +364,17 @@ public class StringToConstraintsTransformer {
      handleOption(constraint, text.toString(), "", invert);
    }
    else if (invert) throw new MalformedPatternException(SSRBundle.message("error.expected.condition", "!"));
-    else if (optionExpected) throw new MalformedPatternException(SSRBundle.message("error.expected.condition", "&&"));
+    else if (optionExpected) throw new MalformedPatternException(SSRBundle.message("error.expected.condition", length == 0 ? "[" : "&&"));
  }

  private static void handleOption(@NotNull MatchVariableConstraint constraint, @NotNull String option, @NotNull String argument,
                                   boolean invert) {
    argument = argument.trim();
    if (option.equalsIgnoreCase(REF)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.pattern.argument.expected", option));
      constraint.setReferenceConstraint(argument);
      constraint.setInvertReference(invert);
    }
    else if (option.equalsIgnoreCase(REGEX) || option.equalsIgnoreCase(REGEXW)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.regular.expression.argument.expected", option));
      if (argument.charAt(0) == '*') {
        argument = argument.substring(1);
        constraint.setWithinHierarchy(true);
@@ -374,7 +387,6 @@ public class StringToConstraintsTransformer {
      }
    }
    else if (option.equalsIgnoreCase(EXPRTYPE)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.regular.expression.argument.expected", option));
      if (argument.charAt(0) == '*') {
        argument = argument.substring(1);
        constraint.setExprTypeWithinHierarchy(true);
@@ -384,7 +396,6 @@ public class StringToConstraintsTransformer {
      constraint.setInvertExprType(invert);
    }
    else if (option.equalsIgnoreCase(FORMAL)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.regular.expression.argument.expected", option));
      if (argument.charAt(0) == '*') {
        argument = argument.substring(1);
        constraint.setFormalArgTypeWithinHierarchy(true);
@@ -394,24 +405,21 @@ public class StringToConstraintsTransformer {
      constraint.setInvertFormalType(invert);
    }
    else if (option.equalsIgnoreCase(SCRIPT)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.script.argument.expected", option));
      if (invert) throw new MalformedPatternException(SSRBundle.message("error.cannot.invert", option));
      constraint.setScriptCodeConstraint(argument);
    }
    else if (option.equalsIgnoreCase(CONTAINS)) {
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.pattern.argument.expected", option));
      constraint.setContainsConstraint(argument);
      constraint.setInvertContainsConstraint(invert);
    }
    else if (option.equalsIgnoreCase(WITHIN)) {
      if (!Configuration.CONTEXT_VAR_NAME.equals(constraint.getName()))
        throw new MalformedPatternException(SSRBundle.message("error.only.applicable.to.complete.match", option));
-      if (argument.isEmpty()) throw new MalformedPatternException(SSRBundle.message("error.pattern.argument.expected", option));
      constraint.setWithinConstraint(argument);
      constraint.setInvertWithinConstraint(invert);
    }
    else {
-      throw new UnsupportedPatternException(SSRBundle.message("option.is.not.recognized.error.message", option));
+      assert false;
    }
  }

@@ -209,23 +209,20 @@ option.is.not.recognized.error.message=Constraint ''{0}'' not recognized
 error.only.one.target.allowed=Only one target allowed
 error.condition.only.on.first.variable.reference=Constraints are only allowed on the first reference of a variable
 error.two.different.type.constraints=Two different type constraints
-error.incorrect.regexp.constraint=Incorrect reg exp constraint:{0} for {1}
+error.incorrect.regexp.constraint=Incorrect reg exp constraint: {0} for {1}
 error.expected.character=Character expected after single quote
 error.overflow=Value overflow
+error.expected.digit=Digit expected
 error.expected.brace1=Digit, '}' or ',' expected
 error.expected.brace2=Digit or '}' expected
+error.empty.quantifier=Empty quantifier
 error.expected.condition=Constraint expected after ''{0}''
-error.expected.condition.or.bracket=Constraint or ']' expected
 error.expected.condition.name=Constraint name missing
 error.expected.value=''{0}'' expected
 error.unexpected.value=Unexpected ''{0}''
 invalid.modifier.type=Invalid modifier type {0}
-error.reference.variable.name.expected=Name of reference variable argument expected on ''{0}'' option
-error.no.argument.expected=No argument expected on ''{0}'' option
-error.regular.expression.argument.expected=Regular expression argument expected on ''{0}'' option
-error.pattern.argument.expected=Pattern argument expected on ''{0}'' option
-error.script.argument.expected=Script argument expected on ''{0}'' option
-error.cannot.invert=Cannot invert ''{0}'' option
+error.argument.expected=Argument expected on ''{0}'' constraint
+error.cannot.invert=Cannot invert ''{0}'' constraint
 error.only.applicable.to.complete.match=Constraint ''{0}'' is only applicable to Complete Match

 SSRInspection.replace.with=Replace with ''{0}''
@@ -988,11 +988,11 @@ public class StructuralSearchTest extends StructuralSearchTestCase {
    assertEquals("backtracking greedy regexp", 1, findMatchesCount(s89, "{ '_T*; '_T2*; }"));
    assertEquals("backtracking greedy regexp 2", 1, findMatchesCount(s89, " { '_T*; '_T2*; '_T3+; } "));
    assertEquals("backtracking greedy regexp 3", 0, findMatchesCount(s89, " { '_T+; '_T2+; '_T3+; '_T4+; } "));
-    assertEquals("counted regexp (with back tracking)", 1, findMatchesCount(s89, " { '_T{1,3}; '_T2{2}; } "));
+    assertEquals("counted regexp (with back tracking)", 1, findMatchesCount(s89, " { '_T{1,3}; '_T2{2,}; } "));
    assertEquals("nongreedy regexp (counted, with back tracking)", 1,
-                 findMatchesCount(s89, " { '_T{1}?; '_T2*?; '_T3+?; } "));
+                 findMatchesCount(s89, " { '_T{1,}?; '_T2*?; '_T3+?; } "));
    assertEquals("nongreedy regexp (counted, with back tracking) 2", 0,
-                 findMatchesCount(s89, " { '_T{1}?; '_T2{1,2}?; '_T3+?; '_T4+?; } "));
+                 findMatchesCount(s89, " { '_T{1,}?; '_T2{1,2}?; '_T3+?; '_T4+?; } "));

    String s1000 = "class A {\n" +
                   "      void _() {}\n" +
@@ -1356,7 +1356,7 @@ public class StructuralSearchTest extends StructuralSearchTestCase {
    try {
      findMatchesCount(s109, "'_:*[regex( I ) && ref2('T)] '_;");
      fail("implements navigation match in definition 2 with nested conditions - incorrect cond");
-    } catch(UnsupportedPatternException ignored) {}
+    } catch (MalformedPatternException ignored) {}

    final String s111 = "interface E {} class A implements E {} class B extends A { int f = 0; } class C extends B {} class D { void e() { C c; B b; A a;} }";
    final String s112 = "'_";
@@ -4,11 +4,15 @@ package com.intellij.structuralsearch.impl.matcher.compiler;
 import com.intellij.structuralsearch.MalformedPatternException;
 import com.intellij.structuralsearch.MatchOptions;
 import com.intellij.structuralsearch.MatchVariableConstraint;
-import com.intellij.structuralsearch.UnsupportedPatternException;
 import com.intellij.structuralsearch.plugin.ui.Configuration;
+import org.hamcrest.CoreMatchers;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;

+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.equalTo;
 import static org.junit.Assert.*;

 /**
@@ -16,6 +20,9 @@ import static org.junit.Assert.*;
 */
 public class StringToConstraintsTransformerTest {

+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
  private MatchOptions myOptions;

  @Before
@@ -23,52 +30,68 @@ public class StringToConstraintsTransformerTest {
    myOptions = new MatchOptions();
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testCharacterExpectedAfterQuote() {
-    test("' asdf");
-  }
-
-  @Test(expected = MalformedPatternException.class)
-  public void testCharacterExpectedAfterQuote2() {
-    test("'");
-  }
-
-  @Test(expected = MalformedPatternException.class)
-  public void testUnexpectedEndOfPattern() {
-    test("'_a{");
-  }
-
-  @Test(expected = MalformedPatternException.class)
-  public void testDigitExpected() {
-    test("'a{a");
-  }
-
-  @Test(expected = MalformedPatternException.class)
-  public void testDigitExpected2() {
-    test("'a{1,a}");
+    expectException("' asdf", "Character expected after single quote");
  }

  @Test
-  public void testZeroOccurs() {
-    test("'a{,}");
-    final MatchVariableConstraint constraint = myOptions.getVariableConstraint("a");
+  public void testCharacterExpectedAfterQuote2() {
+    expectException("'", "Character expected after single quote");
+  }
+
+  @Test
+  public void testUnexpectedEndOfPattern() {
+    expectException("'_a{", "Digit expected");
+  }
+
+  @Test
+  public void testDigitExpected() {
+    expectException("'a{a", "Digit expected");
+  }
+
+  @Test
+  public void testDigitExpected2() {
+    expectException("'a{1,a}", "Digit, '}' or ',' expected");
+  }
+
+  @Test
+  public void testCountedOccurs() {
+    test("'_a{3,}'_b{4} '_c{,5}");
+    MatchVariableConstraint constraint = myOptions.getVariableConstraint("a");
+    assertEquals(3, constraint.getMinCount());
+    assertEquals(Integer.MAX_VALUE, constraint.getMaxCount());
+    constraint = myOptions.getVariableConstraint("b");
+    assertEquals(4, constraint.getMinCount());
+    assertEquals(4, constraint.getMaxCount());
+    constraint = myOptions.getVariableConstraint("c");
    assertEquals(0, constraint.getMinCount());
-    assertEquals(0, constraint.getMaxCount());
+    assertEquals(5, constraint.getMaxCount());
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
+  public void testEmptyQuantifier1() {
+    expectException("'_a{}", "Empty quantifier");
+  }
+
+  @Test
+  public void testEmptyQuantifier2() {
+    expectException("'_a{,}", "Empty quantifier");
+  }
+
+  @Test
  public void testOverflow() {
-    test("'a{2147483648}");
+    expectException("'a{2147483648}", "Value overflow");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testMissingBrace() {
-    test("'a{1,3");
+    expectException("'a{1,3", "Digit or '}' expected");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testNoOptions() {
-    test("'a:");
+    expectException("'a:", "Constraint expected after ':'");
  }

  @Test
@@ -77,59 +100,61 @@ public class StringToConstraintsTransformerTest {
    assertEquals("for($t$ $a$ : $b$) {}", myOptions.getSearchPattern());
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testNoOptions2() {
-    test("'a:+");
+    expectException("'a:+", "Constraint expected after '+'");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testUnclosedCondition() {
-    test("'a:[");
+    expectException("'a:[", "']' expected");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testClosedCondition() {
-    test("'a:[]");
+    expectException("'a:[]", "Constraint expected after '['");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testEmptyNegated() {
-    test("'a:[!]");
+    expectException("'a:[!]", "Constraint expected after '!'");
  }

-  @Test(expected = UnsupportedPatternException.class)
+  @Test
  public void testCondition() {
-    test("'a:[aap()]");
+    expectException("'a:[aap()]", "Constraint 'aap' not recognized");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testIncompleteCondition() {
-    test("'a:[regex(]");
+    expectException("'a:[regex(]", "Argument expected on 'regex' constraint");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testIncompleteCondition2() {
-    test("'a:[regex()]");
+    expectException("'a:[regex()]", "Argument expected on 'regex' constraint");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testIncompleteMultipleCondition() {
-    test("'a:[regex( a ) &&]");
+    expectException("'a:[regex( a ) &&]", "Constraint expected after '&&'");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testInvalidRegularExpression() {
-    test("'a:x!(");
+    expectException("'a:x!(", "Invalid regular expression: Unclosed group near index 3\n" +
+                              "x!(\n" +
+                              "   ^");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testRepeatingConstraints() {
-    test("'a*:foo 'a+:[regex( bla )]");
+    expectException("'a*:foo 'a+:[regex( bla )]", "Constraints are only allowed on the first reference of a variable");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testRepeatingConstraints2() {
-    test("'a:foo 'a*");
+    expectException("'a:foo 'a*", "Constraints are only allowed on the first reference of a variable");
  }

  @Test
@@ -146,9 +171,9 @@ public class StringToConstraintsTransformerTest {
    assertEquals("\"if('_a) { 'st*; }\"", constraint.getWithinConstraint());
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testBadWithin() {
-    test("'_type 'a:[within( \"if ('_a) { '_st*; }\" )] = '_b;");
+    expectException("'_type 'a:[within( \"if ('_a) { '_st*; }\" )] = '_b;", "Constraint 'within' is only applicable to Complete Match");
  }

  @Test
@@ -235,24 +260,25 @@ public class StringToConstraintsTransformerTest {
    assertEquals("A", constraint.getRegExp());
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testAmpersandsExpected() {
-    test("'a:[regex(a) regex(b)]");
+    expectException("'a:[regex(a) regex(b)]", "'&&' expected");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testUnexpectedAmpersands() {
-    test("'a:[&&regex(a)]");
+    expectException("'a:[&&regex(a)]", "Unexpected '&'");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testUnbalancedSpacesSurroundingContent() {
-    test("'a:[regex(  .* ) ]");
+    expectException("'a:[regex(  .* ) ]", "'  )' expected");
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testInvalidRegex() {
-    test("'T:{ ;");
+    expectException("'T:{ ;", "Invalid regular expression: Illegal repetition\n" +
+                              "{");
  }

  @Test
@@ -262,9 +288,9 @@ public class StringToConstraintsTransformerTest {
    assertEquals("a", constraint.getRegExp());
  }

-  @Test(expected = MalformedPatternException.class)
+  @Test
  public void testMultipleTargets() {
-    test("try { 'Statements+; } catch('_ '_) { 'HandlerStatements+; }");
+    expectException("try { 'Statements+; } catch('_ '_) { 'HandlerStatements+; }", "Only one target allowed");
  }

  @Test
@@ -272,6 +298,12 @@ public class StringToConstraintsTransformerTest {
    test("'a = 'a;");
  }

+  private void expectException(String criteria, String exceptionMessage) {
+    thrown.expect(MalformedPatternException.class);
+    thrown.expectMessage(equalTo(exceptionMessage));
+    test(criteria);
+  }
+
  private void test(String criteria) {
    StringToConstraintsTransformer.transformCriteria(criteria, myOptions);
  }