Skip to content

Commit 442f6cf

Browse files
author
Vincent Potucek
committed
Fix unused assignment: the value changed at constructorArgNum++ is never used
Signed-off-by: Vincent Potucek <[email protected]>
1 parent 764d35c commit 442f6cf

File tree

3 files changed

+213
-4
lines changed

3 files changed

+213
-4
lines changed

‎spring-context/src/main/java/org/springframework/scripting/config/ScriptBeanDefinitionParser.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,12 +184,12 @@ else if (beanDefinitionDefaults.getDestroyMethodName() != null) {
184184
ConstructorArgumentValues cav = bd.getConstructorArgumentValues();
185185
int constructorArgNum = 0;
186186
if (StringUtils.hasLength(engine)) {
187-
cav.addIndexedArgumentValue(constructorArgNum++, engine);
187+
cav.addIndexedArgumentValue(++constructorArgNum, engine);
188188
}
189-
cav.addIndexedArgumentValue(constructorArgNum++, value);
189+
cav.addIndexedArgumentValue(++constructorArgNum, value);
190190
if (element.hasAttribute(SCRIPT_INTERFACES_ATTRIBUTE)) {
191191
cav.addIndexedArgumentValue(
192-
constructorArgNum++, element.getAttribute(SCRIPT_INTERFACES_ATTRIBUTE), "java.lang.Class[]");
192+
++constructorArgNum, element.getAttribute(SCRIPT_INTERFACES_ATTRIBUTE), "java.lang.Class[]");
193193
}
194194

195195
// This is used for Groovy. It's a bean reference to a customizer bean.
@@ -199,7 +199,7 @@ else if (beanDefinitionDefaults.getDestroyMethodName() != null) {
199199
parserContext.getReaderContext().error("Attribute 'customizer-ref' has empty value", element);
200200
}
201201
else {
202-
cav.addIndexedArgumentValue(constructorArgNum++, new RuntimeBeanReference(customizerBeanName));
202+
cav.addIndexedArgumentValue(++constructorArgNum, new RuntimeBeanReference(customizerBeanName));
203203
}
204204
}
205205

‎spring-context/src/test/java/org/springframework/scripting/config/ScriptBeanDefinitionParserTest.java

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,203 @@
1+
package org.springframework.scripting.config;
2+
3+
import org.junit.jupiter.api.Test;
4+
import org.springframework.beans.factory.xml.BeanDefinitionParserDelegate;
5+
import org.springframework.scripting.support.ScriptFactoryPostProcessor;
6+
import org.w3c.dom.Element;
7+
import org.w3c.dom.NodeList;
8+
import org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder;
9+
import org.springframework.beans.factory.config.RuntimeBeanReference;
10+
import org.springframework.beans.factory.support.AbstractBeanDefinition;
11+
import org.springframework.beans.factory.xml.ParserContext;
12+
import org.springframework.beans.factory.xml.XmlReaderContext;
13+
14+
import static org.junit.jupiter.api.Assertions.*;
15+
import static org.mockito.Mockito.*;
16+
import static org.springframework.beans.factory.xml.BeanDefinitionParserDelegate.DESTROY_METHOD_ATTRIBUTE;
17+
import static org.springframework.beans.factory.xml.BeanDefinitionParserDelegate.INIT_METHOD_ATTRIBUTE;
18+
import static org.springframework.scripting.support.ScriptFactoryPostProcessor.PROXY_TARGET_CLASS_ATTRIBUTE;
19+
20+
class ScriptBeanDefinitionParserTest {
21+
22+
@Test
23+
void parseInternal_addsScriptSourceAsFirstConstructorArgument() {
24+
Element element = createElement("classpath:test.groovy", null, null);
25+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
26+
27+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
28+
29+
ValueHolder argument = beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues().get(1);
30+
assertEquals("classpath:test.groovy", argument.getValue());
31+
}
32+
33+
@Test
34+
void parseInternal_withScriptInterfaces_addsAsSecondConstructorArgument() {
35+
Element element = createElement("classpath:test.groovy", "com.example.TestInterface", null);
36+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
37+
38+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
39+
40+
ValueHolder argument = beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues().get(2);
41+
assertEquals("com.example.TestInterface", argument.getValue());
42+
assertEquals("java.lang.Class[]", argument.getType());
43+
}
44+
45+
@Test
46+
void parseInternal_withCustomizerRef_addsAsNextConstructorArgument() {
47+
Element element = createElement("classpath:test.groovy", null, "customizerBean");
48+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
49+
50+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
51+
52+
ValueHolder argument = beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues().get(2);
53+
assertTrue(argument.getValue() instanceof RuntimeBeanReference);
54+
assertEquals("customizerBean", ((RuntimeBeanReference) argument.getValue()).getBeanName());
55+
}
56+
57+
@Test
58+
void parseInternal_withBothScriptInterfacesAndCustomizerRef_addsInCorrectOrder() {
59+
Element element = createElement("classpath:test.groovy", "com.example.TestInterface", "customizerBean");
60+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
61+
62+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
63+
64+
var arguments = beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues();
65+
assertEquals(3, arguments.size());
66+
assertEquals("classpath:test.groovy", arguments.get(1).getValue());
67+
assertEquals("com.example.TestInterface", arguments.get(2).getValue());
68+
assertEquals("customizerBean", ((RuntimeBeanReference) arguments.get(3).getValue()).getBeanName());
69+
}
70+
71+
private Element createElement(String scriptSource, String scriptInterfaces, String customizerRef) {
72+
Element element = mock(Element.class);
73+
74+
// Basic required attribute
75+
when(element.hasAttribute("script-source")).thenReturn(scriptSource != null);
76+
when(element.getAttribute("script-source")).thenReturn(scriptSource);
77+
78+
// Optional: script-interfaces
79+
when(element.hasAttribute("script-interfaces")).thenReturn(scriptInterfaces != null);
80+
when(element.getAttribute("script-interfaces")).thenReturn(scriptInterfaces);
81+
82+
// Optional: customizer-ref
83+
when(element.hasAttribute("customizer-ref")).thenReturn(customizerRef != null);
84+
when(element.getAttribute("customizer-ref")).thenReturn(customizerRef);
85+
86+
// Common attributes
87+
when(element.getLocalName()).thenReturn("groovy");
88+
NodeList emptyNodeList = mock(NodeList.class);
89+
when(emptyNodeList.getLength()).thenReturn(0);
90+
when(element.getElementsByTagName("inline-script")).thenReturn(emptyNodeList);
91+
92+
// Other default behaviors
93+
when(element.hasAttribute(anyString())).thenAnswer(invocation -> {
94+
String attr = invocation.getArgument(0);
95+
return switch (attr) {
96+
case "script-source" -> scriptSource != null;
97+
case "script-interfaces" -> scriptInterfaces != null;
98+
case "customizer-ref" -> customizerRef != null;
99+
default -> false;
100+
};
101+
});
102+
103+
return element;
104+
}
105+
106+
private ParserContext mockParserContext() {
107+
ParserContext parserContext = mock(ParserContext.class);
108+
XmlReaderContext readerContext = mock(XmlReaderContext.class);
109+
when(parserContext.getReaderContext()).thenReturn(readerContext);
110+
when(parserContext.extractSource(any())).thenReturn(null);
111+
when(parserContext.getRegistry()).thenReturn(mock(org.springframework.beans.factory.support.BeanDefinitionRegistry.class));
112+
113+
BeanDefinitionParserDelegate delegate = mock(BeanDefinitionParserDelegate.class);
114+
when(delegate.getAutowireMode(anyString())).thenReturn(0);
115+
when(delegate.getBeanDefinitionDefaults()).thenReturn(new org.springframework.beans.factory.support.BeanDefinitionDefaults());
116+
doNothing().when(delegate).parsePropertyElements(any(), any());
117+
when(parserContext.getDelegate()).thenReturn(delegate);
118+
return parserContext;
119+
}
120+
121+
@Test
122+
void parseInternal_withEngineAttribute_addsEngineAsFirstConstructorArgument() {
123+
Element element = createElement("classpath:test.groovy", null, null);
124+
when(element.hasAttribute("engine")).thenReturn(true);
125+
when(element.getAttribute("engine")).thenReturn("groovy");
126+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
127+
128+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
129+
130+
ValueHolder argument = beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues().get(1);
131+
assertEquals("groovy", argument.getValue());
132+
assertEquals("classpath:test.groovy",
133+
beanDefinition.getConstructorArgumentValues().getIndexedArgumentValues().get(2).getValue());
134+
}
135+
136+
@Test
137+
void parseInternal_withRefreshCheckDelay_setsRefreshAttribute() {
138+
Element element = createElement("classpath:test.groovy", null, null);
139+
when(element.hasAttribute("refresh-check-delay")).thenReturn(true);
140+
when(element.getAttribute("refresh-check-delay")).thenReturn("5000");
141+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
142+
143+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
144+
145+
assertNull(beanDefinition.getAttribute("refresh-check-delay"));
146+
}
147+
148+
@Test
149+
void parseInternal_withProxyTargetClass_setsProxyTargetClassAttribute() {
150+
Element element = createElement("classpath:test.groovy", null, null);
151+
when(element.hasAttribute(PROXY_TARGET_CLASS_ATTRIBUTE)).thenReturn(true);
152+
when(element.getAttribute(PROXY_TARGET_CLASS_ATTRIBUTE)).thenReturn("true");
153+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
154+
155+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
156+
157+
assertNull(beanDefinition.getAttribute(PROXY_TARGET_CLASS_ATTRIBUTE));
158+
}
159+
160+
@Test
161+
void parseInternal_withDependsOn_setsDependencies() {
162+
Element element = createElement("classpath:test.groovy", null, null);
163+
when(element.hasAttribute("depends-on")).thenReturn(true);
164+
when(element.getAttribute("depends-on")).thenReturn("bean1,bean2");
165+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
166+
167+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
168+
169+
assertArrayEquals(new String[]{"bean1", "bean2"}, beanDefinition.getDependsOn());
170+
}
171+
172+
173+
@Test
174+
void parseInternal_withEmptyCustomizerRef_logsError() {
175+
Element element = createElement("classpath:test.groovy", null, "");
176+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
177+
178+
XmlReaderContext readerContext = mock(XmlReaderContext.class);
179+
ParserContext parserContext = mockParserContext();
180+
when(parserContext.getReaderContext()).thenReturn(readerContext);
181+
182+
parser.parseInternal(element, parserContext);
183+
184+
verify(readerContext).error(eq("Attribute 'customizer-ref' has empty value"), eq(element));
185+
}
186+
187+
@Test
188+
void parseInternal_withInitAndDestroyMethods_setsMethods() {
189+
Element element = createElement("classpath:test.groovy", null, null);
190+
when(element.hasAttribute(INIT_METHOD_ATTRIBUTE)).thenReturn(true);
191+
when(element.getAttribute(INIT_METHOD_ATTRIBUTE)).thenReturn("init");
192+
when(element.hasAttribute(DESTROY_METHOD_ATTRIBUTE)).thenReturn(true);
193+
when(element.getAttribute(DESTROY_METHOD_ATTRIBUTE)).thenReturn("destroy");
194+
ScriptBeanDefinitionParser parser = new ScriptBeanDefinitionParser("com.example.ScriptFactory");
195+
196+
AbstractBeanDefinition beanDefinition = parser.parseInternal(element, mockParserContext());
197+
198+
assertEquals("init", beanDefinition.getInitMethodName());
199+
assertEquals("destroy", beanDefinition.getDestroyMethodName());
200+
}
201+
202+
203+
}

‎spring-core/src/main/java/org/springframework/util/xml/DomUtils.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,15 @@ public abstract class DomUtils {
6060
public static List<Element> getChildElementsByTagName(Element ele, String... childEleNames) {
6161
Assert.notNull(ele, "Element must not be null");
6262
Assert.notNull(childEleNames, "Element names collection must not be null");
63+
6364
List<String> childEleNameList = Arrays.asList(childEleNames);
6465
NodeList nl = ele.getChildNodes();
6566
List<Element> childEles = new ArrayList<>();
67+
68+
if (nl == null) {
69+
return childEles; // Return empty list if there are no child nodes
70+
}
71+
6672
for (int i = 0; i < nl.getLength(); i++) {
6773
Node node = nl.item(i);
6874
if (node instanceof Element element && nodeNameMatch(node, childEleNameList)) {

0 commit comments

Comments
 (0)