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/12/02 11:32:49 UTC

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

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

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


The following commit(s) were added to refs/heads/master by this push:
     new a972a36  SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation
a972a36 is described below

commit a972a36d81cbef75c1f5ff45df0d5a81977fff2d
Author: Radu Cotescu <17...@users.noreply.github.com>
AuthorDate: Mon Dec 2 12:32:40 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);