SLING-8860 - Issue a warning when data-sly-test is passed a constant value for evaluation
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 @@
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 @@
@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.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 @@
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);