From 6bfc70b61e8e74dd44bdd7f9b4d57f4dc7c15a09 Mon Sep 17 00:00:00 2001
From: Sam Brannen <sam@sambrannen.com>
Date: Tue, 4 Apr 2023 13:29:00 +0200
Subject: [PATCH] Polishing

---
 .../expression/ParserContext.java             |  9 +-
 .../expression/spel/SpelReproTests.java       | 28 ++----
 .../spel/TemplateExpressionParsingTests.java  | 94 +++++++------------
 3 files changed, 46 insertions(+), 85 deletions(-)

diff --git a/spring-expression/src/main/java/org/springframework/expression/ParserContext.java b/spring-expression/src/main/java/org/springframework/expression/ParserContext.java
index 86f1b2ba6f..d8504f1a19 100644
--- a/spring-expression/src/main/java/org/springframework/expression/ParserContext.java
+++ b/spring-expression/src/main/java/org/springframework/expression/ParserContext.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -27,10 +27,11 @@ package org.springframework.expression;
 public interface ParserContext {
 
 	/**
-	 * Whether the expression being parsed is a template. A template expression
-	 * consists of literal text that can be mixed with evaluatable blocks. Some examples:
+	 * Whether the expression being parsed is a template.
+	 * <p>A template consists of literal text that can be mixed with expressions.
+	 * Some examples:
 	 * <pre class="code">
-	 * 	   Some literal text
+	 *     Some literal text
 	 *     Hello #{name.firstName}!
 	 *     #{3 + 4}
 	 * </pre>
diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java
index 2724408332..59e363e04d 100644
--- a/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java
+++ b/spring-expression/src/test/java/org/springframework/expression/spel/SpelReproTests.java
@@ -51,6 +51,7 @@ import org.springframework.expression.MethodResolver;
 import org.springframework.expression.ParserContext;
 import org.springframework.expression.PropertyAccessor;
 import org.springframework.expression.TypedValue;
+import org.springframework.expression.common.TemplateParserContext;
 import org.springframework.expression.spel.standard.SpelExpression;
 import org.springframework.expression.spel.standard.SpelExpressionParser;
 import org.springframework.expression.spel.support.ReflectiveMethodResolver;
@@ -77,6 +78,9 @@ import static org.assertj.core.api.Assertions.assertThatIllegalStateException;
  */
 class SpelReproTests extends AbstractExpressionTests {
 
+	private static final ParserContext DOLLARSQUARE_TEMPLATE_PARSER_CONTEXT = new TemplateParserContext("$[", "]");
+
+
 	@Test
 	void NPE_SPR5661() {
 		evaluate("joinThreeStrings('a',null,'c')", "anullc", String.class);
@@ -193,8 +197,8 @@ class SpelReproTests extends AbstractExpressionTests {
 
 	@Test
 	void NPE_SPR5673() {
-		ParserContext hashes = TemplateExpressionParsingTests.HASH_DELIMITED_PARSER_CONTEXT;
-		ParserContext dollars = TemplateExpressionParsingTests.DEFAULT_TEMPLATE_PARSER_CONTEXT;
+		ParserContext hashes = ParserContext.TEMPLATE_EXPRESSION;
+		ParserContext dollars = TemplateExpressionParsingTests.DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT;
 
 		checkTemplateParsing("abc${'def'} ghi", "abcdef ghi");
 
@@ -378,7 +382,7 @@ class SpelReproTests extends AbstractExpressionTests {
 	}
 
 	private void checkTemplateParsing(String expression, String expectedValue) {
-		checkTemplateParsing(expression, TemplateExpressionParsingTests.DEFAULT_TEMPLATE_PARSER_CONTEXT, expectedValue);
+		checkTemplateParsing(expression, TemplateExpressionParsingTests.DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT, expectedValue);
 	}
 
 	private void checkTemplateParsing(String expression, ParserContext context, String expectedValue) {
@@ -388,7 +392,7 @@ class SpelReproTests extends AbstractExpressionTests {
 	}
 
 	private void checkTemplateParsingError(String expression, String expectedMessage) {
-		checkTemplateParsingError(expression, TemplateExpressionParsingTests.DEFAULT_TEMPLATE_PARSER_CONTEXT, expectedMessage);
+		checkTemplateParsingError(expression, TemplateExpressionParsingTests.DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT, expectedMessage);
 	}
 
 	private void checkTemplateParsingError(String expression, ParserContext context, String expectedMessage) {
@@ -404,22 +408,6 @@ class SpelReproTests extends AbstractExpressionTests {
 			});
 	}
 
-
-	private static final ParserContext DOLLARSQUARE_TEMPLATE_PARSER_CONTEXT = new ParserContext() {
-		@Override
-		public String getExpressionPrefix() {
-			return "$[";
-		}
-		@Override
-		public String getExpressionSuffix() {
-			return "]";
-		}
-		@Override
-		public boolean isTemplate() {
-			return true;
-		}
-	};
-
 	@Test
 	void beanResolution() {
 		StandardEvaluationContext context = new StandardEvaluationContext(new XX());
diff --git a/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java b/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java
index b810aed4ab..ba3b9e67e9 100644
--- a/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java
+++ b/spring-expression/src/test/java/org/springframework/expression/spel/TemplateExpressionParsingTests.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,89 +34,62 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
 /**
  * @author Andy Clement
  * @author Juergen Hoeller
+ * @author Sam Brannen
  */
-public class TemplateExpressionParsingTests extends AbstractExpressionTests {
+class TemplateExpressionParsingTests extends AbstractExpressionTests {
 
-	public static final ParserContext DEFAULT_TEMPLATE_PARSER_CONTEXT = new ParserContext() {
-		@Override
-		public String getExpressionPrefix() {
-			return "${";
-		}
-		@Override
-		public String getExpressionSuffix() {
-			return "}";
-		}
-		@Override
-		public boolean isTemplate() {
-			return true;
-		}
-	};
-
-	public static final ParserContext HASH_DELIMITED_PARSER_CONTEXT = new ParserContext() {
-		@Override
-		public String getExpressionPrefix() {
-			return "#{";
-		}
-		@Override
-		public String getExpressionSuffix() {
-			return "}";
-		}
-		@Override
-		public boolean isTemplate() {
-			return true;
-		}
-	};
+	static final ParserContext DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT = new TemplateParserContext("${", "}");
 
 
 	@Test
-	public void testParsingSimpleTemplateExpression01() throws Exception {
+	void parsingSimpleTemplateExpression01() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
-		Expression expr = parser.parseExpression("hello ${'world'}", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression expr = parser.parseExpression("hello ${'world'}", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		Object o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("hello world");
 	}
 
 	@Test
-	public void testParsingSimpleTemplateExpression02() throws Exception {
+	void parsingSimpleTemplateExpression02() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
-		Expression expr = parser.parseExpression("hello ${'to'} you", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression expr = parser.parseExpression("hello ${'to'} you", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		Object o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("hello to you");
 	}
 
 	@Test
-	public void testParsingSimpleTemplateExpression03() throws Exception {
+	void parsingSimpleTemplateExpression03() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
 		Expression expr = parser.parseExpression("The quick ${'brown'} fox jumped over the ${'lazy'} dog",
-				DEFAULT_TEMPLATE_PARSER_CONTEXT);
+				DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		Object o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("The quick brown fox jumped over the lazy dog");
 	}
 
 	@Test
-	public void testParsingSimpleTemplateExpression04() throws Exception {
+	void parsingSimpleTemplateExpression04() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
-		Expression expr = parser.parseExpression("${'hello'} world", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression expr = parser.parseExpression("${'hello'} world", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		Object o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("hello world");
 
-		expr = parser.parseExpression("", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		expr = parser.parseExpression("", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("");
 
-		expr = parser.parseExpression("abc", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		expr = parser.parseExpression("abc", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		o = expr.getValue();
 		assertThat(o.toString()).isEqualTo("abc");
 
-		expr = parser.parseExpression("abc", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		expr = parser.parseExpression("abc", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		o = expr.getValue((Object)null);
 		assertThat(o.toString()).isEqualTo("abc");
 	}
 
 	@Test
-	public void testCompositeStringExpression() throws Exception {
+	void compositeStringExpression() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
-		Expression ex = parser.parseExpression("hello ${'world'}", DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression ex = parser.parseExpression("hello ${'world'}", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		assertThat(ex.getValue()).isInstanceOf(String.class).isEqualTo("hello world");
 		assertThat(ex.getValue(String.class)).isInstanceOf(String.class).isEqualTo("hello world");
 		assertThat(ex.getValue((Object)null, String.class)).isInstanceOf(String.class).isEqualTo("hello world");
@@ -154,15 +127,15 @@ public class TemplateExpressionParsingTests extends AbstractExpressionTests {
 	static class Rooty {}
 
 	@Test
-	public void testNestedExpressions() throws Exception {
+	void nestedExpressions() throws Exception {
 		SpelExpressionParser parser = new SpelExpressionParser();
 		// treat the nested ${..} as a part of the expression
-		Expression ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} world",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		String s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello 4 world");
 
 		// not a useful expression but tests nested expression syntax that clashes with template prefix/suffix
-		ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#root.listOfNumbersUpToTen.$[#this%2==1]==3]} world",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#root.listOfNumbersUpToTen.$[#this%2==1]==3]} world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		assertThat(ex.getClass()).isEqualTo(CompositeStringExpression.class);
 		CompositeStringExpression cse = (CompositeStringExpression)ex;
 		Expression[] exprs = cse.getExpressions();
@@ -171,60 +144,59 @@ public class TemplateExpressionParsingTests extends AbstractExpressionTests {
 		s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello  world");
 
-		ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} ${listOfNumbersUpToTen.$[#this>5]} world",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		ex = parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} ${listOfNumbersUpToTen.$[#this>5]} world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello 4 10 world");
 
 		assertThatExceptionOfType(ParseException.class).isThrownBy(() ->
-				parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} ${listOfNumbersUpToTen.$[#this>5] world",DEFAULT_TEMPLATE_PARSER_CONTEXT))
+				parser.parseExpression("hello ${listOfNumbersUpToTen.$[#this<5]} ${listOfNumbersUpToTen.$[#this>5] world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT))
 			.satisfies(pex -> assertThat(pex.getSimpleMessage()).isEqualTo("No ending suffix '}' for expression starting at character 41: ${listOfNumbersUpToTen.$[#this>5] world"));
 
 		assertThatExceptionOfType(ParseException.class).isThrownBy(() ->
-				parser.parseExpression("hello ${listOfNumbersUpToTen.$[#root.listOfNumbersUpToTen.$[#this%2==1==3]} world",DEFAULT_TEMPLATE_PARSER_CONTEXT))
+				parser.parseExpression("hello ${listOfNumbersUpToTen.$[#root.listOfNumbersUpToTen.$[#this%2==1==3]} world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT))
 			.satisfies(pex -> assertThat(pex.getSimpleMessage()).isEqualTo("Found closing '}' at position 74 but most recent opening is '[' at position 30"));
 	}
 
 	@Test
-
-	public void testClashingWithSuffixes() throws Exception {
+	void clashingWithSuffixes() throws Exception {
 		// Just wanting to use the prefix or suffix within the template:
-		Expression ex = parser.parseExpression("hello ${3+4} world",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		Expression ex = parser.parseExpression("hello ${3+4} world",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		String s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello 7 world");
 
-		ex = parser.parseExpression("hello ${3+4} wo${'${'}rld",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		ex = parser.parseExpression("hello ${3+4} wo${'${'}rld",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello 7 wo${rld");
 
-		ex = parser.parseExpression("hello ${3+4} wo}rld",DEFAULT_TEMPLATE_PARSER_CONTEXT);
+		ex = parser.parseExpression("hello ${3+4} wo}rld",DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT);
 		s = ex.getValue(TestScenarioCreator.getTestEvaluationContext(),String.class);
 		assertThat(s).isEqualTo("hello 7 wo}rld");
 	}
 
 	@Test
-	public void testParsingNormalExpressionThroughTemplateParser() throws Exception {
+	void parsingNormalExpressionThroughTemplateParser() throws Exception {
 		Expression expr = parser.parseExpression("1+2+3");
 		assertThat(expr.getValue()).isEqualTo(6);
 	}
 
 	@Test
-	public void testErrorCases() throws Exception {
+	void errorCases() throws Exception {
 		assertThatExceptionOfType(ParseException.class).isThrownBy(() ->
-				parser.parseExpression("hello ${'world'", DEFAULT_TEMPLATE_PARSER_CONTEXT))
+				parser.parseExpression("hello ${'world'", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT))
 			.satisfies(pex -> {
 				assertThat(pex.getSimpleMessage()).isEqualTo("No ending suffix '}' for expression starting at character 6: ${'world'");
 				assertThat(pex.getExpressionString()).isEqualTo("hello ${'world'");
 			});
 		assertThatExceptionOfType(ParseException.class).isThrownBy(() ->
-				parser.parseExpression("hello ${'wibble'${'world'}", DEFAULT_TEMPLATE_PARSER_CONTEXT))
+				parser.parseExpression("hello ${'wibble'${'world'}", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT))
 			.satisfies(pex -> assertThat(pex.getSimpleMessage()).isEqualTo("No ending suffix '}' for expression starting at character 6: ${'wibble'${'world'}"));
 		assertThatExceptionOfType(ParseException.class).isThrownBy(() ->
-				parser.parseExpression("hello ${} world", DEFAULT_TEMPLATE_PARSER_CONTEXT))
+				parser.parseExpression("hello ${} world", DOLLAR_SIGN_TEMPLATE_PARSER_CONTEXT))
 			.satisfies(pex -> assertThat(pex.getSimpleMessage()).isEqualTo("No expression defined within delimiter '${}' at character 6"));
 	}
 
 	@Test
-	public void testTemplateParserContext() {
+	void templateParserContext() {
 		TemplateParserContext tpc = new TemplateParserContext("abc","def");
 		assertThat(tpc.getExpressionPrefix()).isEqualTo("abc");
 		assertThat(tpc.getExpressionSuffix()).isEqualTo("def");