Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reintroduce whitespace control parsing fix with legacy override #903

Merged
merged 14 commits into from
Jul 28, 2022
19 changes: 18 additions & 1 deletion src/main/java/com/hubspot/jinjava/LegacyOverrides.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ public class LegacyOverrides {
private final boolean usePyishObjectMapper;
private final boolean whitespaceRequiredWithinTokens;
private final boolean useNaturalOperatorPrecedence;
private final boolean parseWhitespaceControlStrictly;

private LegacyOverrides(Builder builder) {
evaluateMapKeys = builder.evaluateMapKeys;
iterateOverMapKeys = builder.iterateOverMapKeys;
usePyishObjectMapper = builder.usePyishObjectMapper;
whitespaceRequiredWithinTokens = builder.whitespaceRequiredWithinTokens;
useNaturalOperatorPrecedence = builder.useNaturalOperatorPrecedence;
parseWhitespaceControlStrictly = builder.parseWhitespaceControlStrictly;
}

public static Builder newBuilder() {
Expand All @@ -44,12 +46,17 @@ public boolean isUseNaturalOperatorPrecedence() {
return useNaturalOperatorPrecedence;
}

public boolean isParseWhitespaceControlStrictly() {
return parseWhitespaceControlStrictly;
}

public static class Builder {
private boolean evaluateMapKeys = false;
private boolean iterateOverMapKeys = false;
private boolean usePyishObjectMapper = false;
private boolean whitespaceRequiredWithinTokens = false;
private boolean useNaturalOperatorPrecedence = false;
private boolean parseWhitespaceControlStrictly = false;

private Builder() {}

Expand All @@ -65,7 +72,10 @@ public static Builder from(LegacyOverrides legacyOverrides) {
.withWhitespaceRequiredWithinTokens(
legacyOverrides.whitespaceRequiredWithinTokens
)
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence);
.withUseNaturalOperatorPrecedence(legacyOverrides.useNaturalOperatorPrecedence)
.withParseWhitespaceControlStrictly(
legacyOverrides.parseWhitespaceControlStrictly
);
}

public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
Expand Down Expand Up @@ -96,5 +106,12 @@ public Builder withUseNaturalOperatorPrecedence(
this.useNaturalOperatorPrecedence = useNaturalOperatorPrecedence;
return this;
}

public Builder withParseWhitespaceControlStrictly(
boolean parseWhitespaceControlStrictly
) {
this.parseWhitespaceControlStrictly = parseWhitespaceControlStrictly;
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,16 +44,7 @@ public int getType() {
@Override
protected void parse() {
this.expr = WhitespaceUtils.unwrap(image, "{{", "}}");

if (WhitespaceUtils.startsWith(expr, "-")) {
setLeftTrim(true);
this.expr = WhitespaceUtils.unwrap(expr, "-", "");
}
if (WhitespaceUtils.endsWith(expr, "-")) {
setRightTrim(true);
this.expr = WhitespaceUtils.unwrap(expr, "", "-");
}

this.expr = handleTrim(expr);
this.expr = StringUtils.trimToEmpty(this.expr);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package com.hubspot.jinjava.tree.parse;

import com.hubspot.jinjava.util.WhitespaceUtils;

public class LenientWhitespaceControlParser implements WhitespaceControlParser {

@Override
public boolean hasLeftTrim(String unwrapped) {
return WhitespaceUtils.startsWith(unwrapped, "-");
}

@Override
public String stripLeft(String unwrapped) {
return WhitespaceUtils.unwrap(unwrapped, "-", "");
}

@Override
public boolean hasRightTrim(String unwrapped) {
return WhitespaceUtils.endsWith(unwrapped, "-");
}

@Override
public String stripRight(String unwrapped) {
return WhitespaceUtils.unwrap(unwrapped, "", "-");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.hubspot.jinjava.tree.parse;

public class StrictWhitespaceControlParser implements WhitespaceControlParser {

@Override
public boolean hasLeftTrim(String unwrapped) {
return unwrapped.charAt(0) == '-';
}

@Override
public String stripLeft(String unwrapped) {
return unwrapped.substring(1);
}

@Override
public boolean hasRightTrim(String unwrapped) {
return unwrapped.charAt(unwrapped.length() - 1) == '-';
}

@Override
public String stripRight(String unwrapped) {
return unwrapped.substring(0, unwrapped.length() - 1);
}
}
11 changes: 1 addition & 10 deletions src/main/java/com/hubspot/jinjava/tree/parse/TagToken.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
package com.hubspot.jinjava.tree.parse;

import com.hubspot.jinjava.interpret.TemplateSyntaxException;
import com.hubspot.jinjava.util.WhitespaceUtils;

public class TagToken extends Token {
private static final long serialVersionUID = -4927751270481832992L;
Expand Down Expand Up @@ -54,15 +53,7 @@ protected void parse() {
}

content = image.substring(2, image.length() - 2);

if (WhitespaceUtils.startsWith(content, "-")) {
setLeftTrim(true);
content = WhitespaceUtils.unwrap(content, "-", "");
}
if (WhitespaceUtils.endsWith(content, "-")) {
setRightTrim(true);
content = WhitespaceUtils.unwrap(content, "", "-");
}
content = handleTrim(content);

int nameStart = -1, pos = 0, len = content.length();

Expand Down
33 changes: 33 additions & 0 deletions src/main/java/com/hubspot/jinjava/tree/parse/Token.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
**********************************************************************/
package com.hubspot.jinjava.tree.parse;

import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.LegacyOverrides;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.interpret.UnexpectedTokenException;
import java.io.Serializable;

Expand Down Expand Up @@ -83,6 +86,36 @@ public void setRightTrimAfterEnd(boolean rightTrimAfterEnd) {
this.rightTrimAfterEnd = rightTrimAfterEnd;
}

/**
* Handle any whitespace control characters, capturing whether leading or trailing
* whitespace should be stripped.
* @param unwrapped the content of the block stripped of its delimeters
* @return the content stripped of any whitespace control characters.
*/
protected final String handleTrim(String unwrapped) {
boolean parseWhitespaceControlStrictly = JinjavaInterpreter
.getCurrentMaybe()
.map(JinjavaInterpreter::getConfig)
.map(JinjavaConfig::getLegacyOverrides)
.map(LegacyOverrides::isParseWhitespaceControlStrictly)
.orElse(false);

WhitespaceControlParser parser = parseWhitespaceControlStrictly
? WhitespaceControlParser.STRICT
: WhitespaceControlParser.LENIENT;

String result = unwrapped;
if (parser.hasLeftTrim(result)) {
setLeftTrim(true);
result = parser.stripLeft(result);
}
if (parser.hasRightTrim(result)) {
setRightTrim(true);
result = parser.stripRight(result);
}
return result;
}

public int getStartPosition() {
return startPosition;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package com.hubspot.jinjava.tree.parse;

public interface WhitespaceControlParser {
WhitespaceControlParser LENIENT = new LenientWhitespaceControlParser();
WhitespaceControlParser STRICT = new StrictWhitespaceControlParser();

boolean hasLeftTrim(String unwrapped);
String stripLeft(String unwrapped);

boolean hasRightTrim(String unwrapped);
String stripRight(String unwrapped);
}
Original file line number Diff line number Diff line change
Expand Up @@ -343,4 +343,14 @@ public void itInterpretsFilterChainsInOrder() {
assertThat(interpreter.render("{{ 'foo' | upper | replace('O', 'A') }}"))
.isEqualTo("FAA");
}

@Test
public void itInterpretsWhitespaceControl() {
assertThat(interpreter.render(". {%- set x = 5 -%} .")).isEqualTo("..");
}

@Test
public void itInterpretsEmptyExpressions() {
assertThat(interpreter.render("{{}}")).isEqualTo("");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
package com.hubspot.jinjava.interpret;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;

import com.hubspot.jinjava.Jinjava;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.LegacyOverrides;
import java.util.HashMap;
import org.junit.Before;
import org.junit.Test;

public class LegacyWhitespaceControlParsingTest {
Jinjava legacy;
Jinjava modern;

@Before
public void setUp() throws Exception {
legacy =
new Jinjava(
JinjavaConfig.newBuilder().withLegacyOverrides(LegacyOverrides.NONE).build()
);
modern =
new Jinjava(
JinjavaConfig
.newBuilder()
.withLegacyOverrides(
LegacyOverrides.newBuilder().withParseWhitespaceControlStrictly(true).build()
)
.build()
);
}

@Test
public void itInterpretsStandaloneNegatives() {
String template = "{{ -10 }}";

assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
assertThat(modern.render(template, new HashMap<>())).isEqualTo("-10");
}

@Test
public void itInterpretsStandaloneNegativesWithWhitespace() {
String template = "{{ - 10 }}";

assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");

// Somewhat surprising, but this aligns with Jinja
assertThat(modern.render(template, new HashMap<>())).isEqualTo("-10");
}

@Test
public void itErrorsOnTrailingDash() {
String template = "{{ 10- }}";

assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
assertThatExceptionOfType(FatalTemplateErrorsException.class)
.isThrownBy(() -> modern.render(template, new HashMap<>()))
.withMessageContaining("syntax error at position 6");
}

@Test
public void itErrorsOnSpacedTrailingDash() {
String template = "{{ 10 - }}";

assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
assertThatExceptionOfType(FatalTemplateErrorsException.class)
.isThrownBy(() -> modern.render(template, new HashMap<>()))
.withMessageContaining("syntax error at position 7");
}

@Test
public void itErrorsOnSpacedDashesOnBothSides() {
String template = "{{ - 10 - }}";

assertThat(legacy.render(template, new HashMap<>())).isEqualTo("10");
assertThatExceptionOfType(FatalTemplateErrorsException.class)
.isThrownBy(() -> modern.render(template, new HashMap<>()))
.withMessageContaining("syntax error at position 9");
}
}