- IDEA-67725 XPath 2: consecutive comparison is green

- replaced instanceof checks with visitor

plugins/xpath/xpath-lang/src/org/intellij/lang/xpath/validation/ConsecutiveComparisonFix.java

@@ -0,0 +1,62 @@
+/*
+ * Copyright 2000-2011 JetBrains s.r.o.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.intellij.lang.xpath.validation;
+
+import com.intellij.codeInsight.intention.IntentionAction;
+import com.intellij.openapi.editor.Editor;
+import com.intellij.openapi.project.Project;
+import com.intellij.psi.PsiFile;
+import com.intellij.util.IncorrectOperationException;
+import org.intellij.lang.xpath.psi.XPathBinaryExpression;
+import org.intellij.lang.xpath.psi.impl.XPathChangeUtil;
+import org.jetbrains.annotations.NotNull;
+
+class ConsecutiveComparisonFix implements IntentionAction {
+  private final String myReplacement;
+  private final XPathBinaryExpression myExpr;
+
+  public ConsecutiveComparisonFix(String replacement, XPathBinaryExpression expr) {
+    myReplacement = replacement;
+    myExpr = expr;
+  }
+
+  @NotNull
+  @Override
+  public String getText() {
+    return "Replace with '" + myReplacement + "'";
+  }
+
+  @NotNull
+  @Override
+  public String getFamilyName() {
+    return "XPath2";
+  }
+
+  @Override
+  public boolean isAvailable(@NotNull Project project, Editor editor, PsiFile file) {
+    return myExpr.isValid();
+  }
+
+  @Override
+  public void invoke(@NotNull Project project, Editor editor, PsiFile file) throws IncorrectOperationException {
+    myExpr.replace(XPathChangeUtil.createExpression(myExpr, myReplacement));
+  }
+
+  @Override
+  public boolean startInWriteAction() {
+    return true;
+  }
+}

plugins/xpath/xpath-lang/src/org/intellij/lang/xpath/validation/XPathAnnotator.java

@@ -25,6 +25,8 @@ import com.intellij.openapi.util.TextRange;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiReference;
 import com.intellij.psi.util.PsiTreeUtil;
+import org.intellij.lang.xpath.XPath2TokenTypes;
+import org.intellij.lang.xpath.XPathFileType;
 import org.intellij.lang.xpath.context.functions.Function;
 import org.jetbrains.annotations.NotNull;
 
@@ -41,31 +43,89 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
-public final class XPathAnnotator implements Annotator {
+public final class XPathAnnotator extends XPath2ElementVisitor implements Annotator {
+
+  private AnnotationHolder myHolder;
 
   public void annotate(@NotNull PsiElement psiElement, @NotNull AnnotationHolder holder) {
-    if (!(psiElement instanceof XPathElement)) return;
 
-    final ContextProvider contextProvider = ((XPathElement)psiElement).getXPathContext();
+    try {
+      myHolder = holder;
+      psiElement.accept(this);
+    } finally {
+      myHolder = null;
+    }
+  }
 
-    if (psiElement instanceof XPathNodeTest) {
-      checkNodeTest(contextProvider, holder, (XPathNodeTest)psiElement);
-    } else if (psiElement instanceof XPathStep) {
-      checkSillyStep(holder, (XPathStep)psiElement);
-    } else if (psiElement instanceof XPathNodeTypeTest) {
-      checkNodeTypeTest(holder, ((XPathNodeTypeTest)psiElement));
-    } else if (psiElement instanceof XPathFunctionCall) {
-      checkFunctionCall(holder, ((XPathFunctionCall)psiElement), contextProvider);
-    } else if (psiElement instanceof XPathString) {
-      checkString(holder, ((XPathString)psiElement));
-    } else if (psiElement instanceof XPathVariableReference) {
-      checkVariableReference(holder, ((XPathVariableReference)psiElement), contextProvider);
-    } else if (psiElement instanceof XPath2TypeElement) {
-      checkPrefixReferences(holder, (XPath2TypeElement)psiElement, contextProvider);
-    }
-    if (psiElement instanceof XPathExpression) {
-      checkExpression(holder, ((XPathExpression)psiElement));
+  @Override
+  public void visitXPathNodeTest(XPathNodeTest o) {
+    final ContextProvider contextProvider = o.getXPathContext();
+    checkNodeTest(contextProvider, myHolder, o);
+  }
+
+  @Override
+  public void visitXPathStep(XPathStep o) {
+    checkSillyStep(myHolder, o);
+    super.visitXPathStep(o);
+  }
+
+  @Override
+  public void visitXPathNodeTypeTest(XPathNodeTypeTest o) {
+    checkNodeTypeTest(myHolder, o);
+    visitXPathExpression(o);
+  }
+
+  @Override
+  public void visitXPathFunctionCall(XPathFunctionCall o) {
+    final ContextProvider contextProvider = o.getXPathContext();
+    checkFunctionCall(myHolder, o, contextProvider);
+    super.visitXPathFunctionCall(o);
+  }
+
+  @Override
+  public void visitXPathString(XPathString o) {
+    checkString(myHolder, o);
+    super.visitXPathString(o);
+  }
+
+  @Override
+  public void visitXPathVariableReference(XPathVariableReference o) {
+    final ContextProvider contextProvider = o.getXPathContext();
+    checkVariableReference(myHolder, o, contextProvider);
+    super.visitXPathVariableReference(o);
+  }
+
+  @Override
+  public void visitXPath2TypeElement(XPath2TypeElement o) {
+    final ContextProvider contextProvider = o.getXPathContext();
+    checkPrefixReferences(myHolder, o, contextProvider);
+    super.visitXPath2TypeElement(o);
+  }
+
+  @Override
+  public void visitXPathBinaryExpression(final XPathBinaryExpression o) {
+    if (XPath2TokenTypes.COMP_OPS.contains(o.getOperator())) {
+      if (o.getContainingFile().getLanguage() == XPathFileType.XPATH2.getLanguage()) {
+        final XPathExpression operand = o.getLOperand();
+        if (operand instanceof XPathBinaryExpression && XPath2TokenTypes.COMP_OPS.contains(((XPathBinaryExpression)operand).getOperator())) {
+          final Annotation annotation = myHolder.createErrorAnnotation(o, "Consecutive comparison is not allowed in XPath 2");
+
+          final XPathExpression rOperand = o.getROperand();
+          if (rOperand != null) {
+            final String replacement = "(" + operand.getText() + ") " + o.getOperationSign() + " " + rOperand.getText();
+            annotation.registerFix(new ConsecutiveComparisonFix(replacement, o));
+          }
+        }
+      }
     }
+
+    checkExpression(myHolder, o);
+    super.visitXPathBinaryExpression(o);
+  }
+
+  @Override
+  public void visitXPathExpression(XPathExpression o) {
+    checkExpression(myHolder, o);
   }
 
   private static void checkString(AnnotationHolder holder, XPathString string) {

plugins/xpath/xpath-lang/test/org/intellij/lang/xpath/XPath2HighlightingTest.java

@@ -25,6 +25,10 @@ public class XPath2HighlightingTest extends XPath2HighlightingTestBase {
     doXPathHighlighting();
   }
 
+  public void testConsecutiveComparison() throws Throwable {
+    doXPathHighlighting();
+  }
+
   @Override
   protected String getSubPath() {
     return "xpath/highlighting";

plugins/xpath/xpath-lang/testData/xpath/highlighting/consecutiveComparison.xpath2

@@ -0,0 +1 @@
+<error descr="Consecutive comparison is not allowed in XPath 2">0 = 0 = true()</error>