You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ra...@apache.org on 2019/11/27 09:30:21 UTC

[sling-org-apache-sling-scripting-sightly-compiler] branch issue/SLING-8860 created (now e2cb3e1)

This is an automated email from the ASF dual-hosted git repository.

radu pushed a change to branch issue/SLING-8860
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly-compiler.git.


      at e2cb3e1  SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation

This branch includes the following new commits:

     new e2cb3e1  SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[sling-org-apache-sling-scripting-sightly-compiler] 01/01: SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation

Posted by ra...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

radu pushed a commit to branch issue/SLING-8860
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-scripting-sightly-compiler.git

commit e2cb3e1556389fb7155cabb3f4642c9fcedd67d2
Author: Radu Cotescu <ra...@apache.org>
AuthorDate: Wed Nov 27 10:29:59 2019 +0100

    SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation
---
 .../scripting/sightly/impl/plugin/TestPlugin.java  | 25 ++++++++++++++++++++--
 .../sightly/impl/compiler/SightlyCompilerTest.java | 17 +++++++++++++++
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/apache/sling/scripting/sightly/impl/plugin/TestPlugin.java b/src/main/java/org/apache/sling/scripting/sightly/impl/plugin/TestPlugin.java
index 202bfb4..77b20d9 100644
--- a/src/main/java/org/apache/sling/scripting/sightly/impl/plugin/TestPlugin.java
+++ b/src/main/java/org/apache/sling/scripting/sightly/impl/plugin/TestPlugin.java
@@ -20,6 +20,14 @@ package org.apache.sling.scripting.sightly.impl.plugin;
 
 import org.apache.sling.scripting.sightly.compiler.commands.Conditional;
 import org.apache.sling.scripting.sightly.compiler.commands.VariableBinding;
+import org.apache.sling.scripting.sightly.compiler.expression.ExpressionNode;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.ArrayLiteral;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.Atom;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.BinaryOperation;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.BinaryOperator;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.Identifier;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.MapLiteral;
+import org.apache.sling.scripting.sightly.compiler.expression.nodes.NullLiteral;
 import org.apache.sling.scripting.sightly.impl.compiler.PushStream;
 import org.apache.sling.scripting.sightly.compiler.expression.Expression;
 import org.apache.sling.scripting.sightly.impl.compiler.frontend.CompilerContext;
@@ -44,14 +52,27 @@ public class TestPlugin extends AbstractPlugin {
             @Override
             public void beforeElement(PushStream stream, String tagName) {
                 String variableName = decodeVariableName(callInfo);
+                ExpressionNode root = expressionNode.getRoot();
+                boolean constantValueComparison =
+                        root instanceof Atom && !(root instanceof Identifier) ||
+                        root instanceof NullLiteral ||
+                        root instanceof ArrayLiteral ||
+                        root instanceof MapLiteral;
+                if (!constantValueComparison && root instanceof BinaryOperation) {
+                    constantValueComparison = ((BinaryOperation) root).getOperator() == BinaryOperator.CONCATENATE;
+                }
+                if (constantValueComparison) {
+                    stream.warn(new PushStream.StreamMessage("data-sly-test: redundant constant value comparison",
+                            expressionNode.getRawText()));
+                }
                 globalBinding = variableName != null;
                 if (variableName == null) {
                     variableName = compilerContext.generateVariable("testVariable");
                 }
                 if (globalBinding) {
-                    stream.write(new VariableBinding.Global(variableName, expressionNode.getRoot()));
+                    stream.write(new VariableBinding.Global(variableName, root));
                 } else {
-                    stream.write(new VariableBinding.Start(variableName, expressionNode.getRoot()));
+                    stream.write(new VariableBinding.Start(variableName, root));
                 }
                 stream.write(new Conditional.Start(variableName, true));
             }
diff --git a/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyCompilerTest.java b/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyCompilerTest.java
index 902649c..6aff125 100644
--- a/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyCompilerTest.java
+++ b/src/test/java/org/apache/sling/scripting/sightly/impl/compiler/SightlyCompilerTest.java
@@ -23,6 +23,7 @@ import java.io.InputStreamReader;
 import java.io.Reader;
 import java.io.StringReader;
 import java.util.List;
+import java.util.function.Supplier;
 
 import org.apache.sling.scripting.sightly.compiler.CompilationResult;
 import org.apache.sling.scripting.sightly.compiler.CompilationUnit;
@@ -160,6 +161,22 @@ public class SightlyCompilerTest {
         assertEquals(1, compileSource("<div data-sly-text='${\"text\" @ i18nn}'>Replaced</div>").getWarnings().size());
     }
 
+    @Test
+    public void testRedundantDataSlyTest() {
+        assertEquals("data-sly-test with boolean constant should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${true}\">if true</span>").getWarnings().size());
+        assertEquals("data-sly-test with number constant should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${0}\">if true</span>").getWarnings().size());
+        assertEquals("data-sly-test with string constant should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${'a'}\">if true</span>").getWarnings().size());
+        assertEquals("data-sly-test with null literal should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${}\">if true</span>").getWarnings().size());
+        assertEquals("data-sly-test with array literal should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${[1, 2, 3]}\">if true</span>").getWarnings().size());
+        assertEquals("data-sly-test with string concatenation should have raised a warning.", 1,
+                compileSource("<span data-sly-test=\"${properties}}\">if true</span>").getWarnings().size());
+    }
+
     private CompilationResult compileFile(final String file) {
         InputStream stream = this.getClass().getResourceAsStream(file);
         final Reader reader = new InputStreamReader(stream);