Merge pull request #121 from jgreffe/feat/87_facet-removal

Is Stapler Facet even needed?

README.md

@@ -31,4 +31,4 @@ Features include:
 
 ## JEXL
 
-* Syntax error checks on JEXL expressions (if Stapler [Facet](https://www.jetbrains.com/help/idea/facet-page.html) is configured)
+* Syntax error checks on JEXL expressions

src/main/java/org/kohsuke/stapler/idea/JexlInspection.java

@@ -53,8 +53,6 @@ protected ProblemDescriptor[] checkXmlText(XmlText xmlText, InspectionManager ma
     protected ProblemDescriptor[] check(XmlElement psi, InspectionManager manager, String text, TextRange range, boolean onTheFly) {
         if(!(psi.getContainingFile() instanceof JellyFile))
             return EMPTY_ARRAY; // not a jelly script
-        if(!shouldCheck(psi))
-            return EMPTY_ARRAY; // stapler not enabled
 
         int len = text.length();
 
@@ -73,7 +71,7 @@ protected ProblemDescriptor[] check(XmlElement psi, InspectionManager manager, S
             return new ProblemDescriptor[] {
                 manager.createProblemDescriptor(psi,
                         new TextRange(startIndex, len),
-                        "Missing '}' character at the end of expression: ",
+                        "Missing '}' character at the end of expression",
                         ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
                         onTheFly,
                         LocalQuickFix.EMPTY_ARRAY)
@@ -172,7 +170,7 @@ protected ProblemDescriptor[] check(XmlElement psi, InspectionManager manager, S
                             return new ProblemDescriptor[] {
                                 manager.createProblemDescriptor(psi,
                                         new TextRange(cur - expr.length() - 2, cur + 1).shiftRight(range.getStartOffset()),
-                                        "Missing '}' character at the end of expression: ",
+                                        "Missing '}' character at the end of expression",
                                         ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
                                         onTheFly,
                                         LocalQuickFix.EMPTY_ARRAY)
@@ -230,11 +228,12 @@ private ProblemDescriptor parseJexl(InspectionManager manager, XmlElement psi, T
                     if(token==null)
                         return manager.createProblemDescriptor(psi,range,"Missing ')' at the end",
                             ProblemHighlightType.GENERIC_ERROR_OR_WARNING, onTheFly, LocalQuickFix.EMPTY_ARRAY);
-
+
+                    int updatedOffset = offset-(expr.length()-token.length());
                     try {
                         ExpressionFactory.createExpression(token);
                     } catch (ParseException e) {
-                        return handleParseException(manager, psi, e, offset, onTheFly);
+                        return handleParseException(manager, psi, e, updatedOffset, onTheFly);
                     }
                     expr = expr.substring(token.length()+1);
                     offset += token.length()+1;
@@ -280,7 +279,8 @@ private ProblemDescriptor handleParseException(InspectionManager manager, XmlEle
                     ProblemHighlightType.GENERIC_ERROR_OR_WARNING, onTheFly, LocalQuickFix.EMPTY_ARRAY);
     }
 
-    private String tokenize(String text) {
+    @SuppressWarnings("fallthrough")
+    protected String tokenize(String text) {
         int parenthesis=0;
         for(int idx=0;idx<text.length();idx++) {
             char ch = text.charAt(idx);
@@ -306,6 +306,9 @@ private String tokenize(String text) {
             case '\'':
                 // skip strings
                 idx = text.indexOf(ch,idx+1);
+                if(idx<0) { // avoids infinite loop
+                    return null;
+                }
                 break;
             }
         }

src/main/java/org/kohsuke/stapler/idea/LocalXmlInspectionTool.java

@@ -18,18 +18,6 @@
  * @author Kohsuke Kawaguchi
  */
 public abstract class LocalXmlInspectionTool extends LocalInspectionTool {
-    /**
-     * Returns true if local inspection should kick in,
-     * which is when the stapler facet is configured on the current module.
-     */
-    protected final boolean shouldCheck(@NotNull PsiElement psiElement) {
-        return getFacet(psiElement) != null;
-    }
-
-    @Nullable
-    protected StaplerFacet getFacet(@NotNull PsiElement psiElement) {
-        return StaplerFacet.findFacetBySourceFile(psiElement.getProject(), psiElement.getContainingFile().getVirtualFile());
-    }
 
     @Override
     @NotNull

src/main/resources/META-INF/plugin.xml

@@ -15,10 +15,6 @@
       check the ExtensionPointName.create call in the extension point, and use its name
       as the element name (modulo the part defined in @defaultExtensionNs above.)
     -->
-    <!-- Adds "Stapler" as a framework to the new Project setup wizard -->
-    <frameworkSupport implementation="org.kohsuke.stapler.idea.StaplerFrameworkSupport"/>
-    <!-- Adds module Facet for Stapler -->
-    <facetType implementation="org.kohsuke.stapler.idea.StaplerFacetType"/>
     <!-- Adds "*.jelly" to the XML file type extensions -->
     <!-- *.jellytag is a seldom used stapler jelly tag tag file extension
          https://github.com/jenkinsci/stapler/blob/1709.ve4c10835694b_/jelly/src/main/java/org/kohsuke/stapler/jelly/CustomTagLibrary.java#L128-L130 -->
@@ -137,8 +133,6 @@
   <depends>com.intellij.modules.xml</depends>
   <!-- Stapler is a Java Framework. And this is required to maintain compatibility past 2019.2 -->
   <depends>com.intellij.modules.java</depends>
-  <!-- needed for defining web facet -->
-  <!--depends>com.intellij.javaee</depends-->
   <!-- needed for i18n support -->
   <depends>com.intellij.properties</depends>
   <!-- this is needed for language injection -->

src/test/java/org/kohsuke/stapler/idea/JexlInspectionTest.java

@@ -1,34 +1,19 @@
 package org.kohsuke.stapler.idea;
 
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import com.intellij.codeInsight.daemon.impl.HighlightInfo;
-import com.intellij.facet.FacetManager;
 import com.intellij.lang.annotation.HighlightSeverity;
-import com.intellij.openapi.module.Module;
-import com.intellij.openapi.roots.ContentEntry;
-import com.intellij.openapi.roots.ModifiableRootModel;
-import com.intellij.testFramework.LightProjectDescriptor;
 import com.intellij.testFramework.fixtures.BasePlatformTestCase;
-import com.intellij.testFramework.fixtures.DefaultLightProjectDescriptor;
-import org.jetbrains.annotations.NotNull;
-
-import java.util.List;
 
 public class JexlInspectionTest extends BasePlatformTestCase {
     @Override
     protected String getTestDataPath() {
         return "src/test/testData";
     }
 
-    @Override
-    protected LightProjectDescriptor getProjectDescriptor() {
-        return new DefaultLightProjectDescriptor() {
-            @Override
-            public void configureModule(@NotNull Module module, @NotNull ModifiableRootModel model, @NotNull ContentEntry contentEntry) {
-                FacetManager.getInstance(module).addFacet(StaplerFacetType.INSTANCE, StaplerFacetType.INSTANCE.getDefaultFacetName(), null);
-            }
-        };
-    }
-
     public void testSmokeJexlInspection() {
         myFixture.configureByFile(getTestName(true) + ".jelly");
         myFixture.enableInspections(new JexlInspection());
@@ -43,4 +28,48 @@ public void testParserFailureJexlInspection() {
         assertNotEmpty(highlightInfos);
         assertEquals("Two invalid JEXL expressions are used", 2, highlightInfos.size());
     }
+
+    public void testI18nLookups() {
+        myFixture.configureByFile(getTestName(true) + ".jelly");
+        myFixture.enableInspections(new JexlInspection());
+        List<HighlightInfo> highlightInfos = myFixture.doHighlighting(HighlightSeverity.WEAK_WARNING);
+        assertNotEmpty(highlightInfos);
+        assertEquals("Expected error", 2, highlightInfos.size());
+        assertEquals("Missing '}' character at the end of expression", highlightInfos.get(0).getDescription());
+        assertTrue("Should match error", highlightInfos.get(1).getDescription().contains("Expecting \"(\" ..."));
+    }
+
+    public void testTokenize() {
+        Map<String, String> expectations = new HashMap<>() {
+            {
+                put("${}", null);
+                put("{}", null);
+                put("()", null);
+                put("[]", null);
+                put(",", "");
+                put("{,}", null);
+                put("(,)", null);
+                put("[,]", null);
+                put("test)", "test");
+                put("test,", "test");
+                put("(test,)", null);
+                put("\"", null);
+                put("'", null);
+                put("''", null);
+                put("'test'", null);
+                put("\"test\"", null);
+                put("'test", null);
+                put("\"test", null);
+                put("test'", null);
+                put("test\"", null);
+            }
+        };
+
+        JexlInspection jexlInspection = new JexlInspection();
+
+        for (Map.Entry<String, String> expectation : expectations.entrySet()) {
+            assertEquals("Issue with text: " + expectation.getKey(), expectation.getValue(),
+                         jexlInspection.tokenize(expectation.getKey()));
+        }
+    }
 }

src/test/testData/i18nLookups.jelly

@@ -0,0 +1,6 @@
+<j:jelly xmlns:j="jelly:core">
+    <p>${%Name}</p>
+    <p>${%Build Queue(items.size())}</p>
+    <p>${%Failing</p>
+    <p>${%Failing(items.size)}</p>
+</j:jelly>