Skip to content

Commit 0ced5d3

Browse files
jmgrassaujmgrassau
committed
add options to 'Add space between parentheses and char. literals' (#20)
1 parent 8b10652 commit 0ced5d3

File tree

3 files changed

+307
-10
lines changed

3 files changed

+307
-10
lines changed

‎com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/parser/Token.java

+4
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,10 @@ public final boolean isFloatLiteral() {
153153

154154
final boolean getMayBeIdentifier() { return (type == TokenType.IDENTIFIER) || isKeyword() || isTextualComparisonOp(); }
155155

156+
public final boolean isCharacterLiteral() {
157+
return isLiteral() && !StringUtil.isNullOrEmpty(text) && (text.charAt(0) == ABAP.QUOT_MARK || text.charAt(0) == ABAP.QUOT_MARK2);
158+
}
159+
156160
public final boolean isStringLiteral() {
157161
return isLiteral() && !StringUtil.isNullOrEmpty(text)
158162
&& (text.charAt(0) == ABAP.QUOT_MARK || text.charAt(0) == ABAP.QUOT_MARK2 || text.charAt(0) == ABAP.PIPE || text.charAt(0) == ABAP.BRACE_CLOSE);

‎com.sap.adt.abapcleaner/src/com/sap/adt/abapcleaner/rules/spaces/SpacesInEmptyBracketsRule.java

+94-10
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@
66
import com.sap.adt.abapcleaner.rulebase.*;
77

88
/**
9-
* Ensures that empty parentheses ( ) contain a single space only
10-
*
9+
* Removes multiple spaces from empty parentheses and adds missing spaces between parentheses and character literals.
1110
*/
1211
public class SpacesInEmptyBracketsRule extends RuleForTokens {
1312
private final static RuleReference[] references = new RuleReference[] {
@@ -20,10 +19,10 @@ public class SpacesInEmptyBracketsRule extends RuleForTokens {
2019
public RuleGroupID getGroupID() { return RuleGroupID.SPACES; }
2120

2221
@Override
23-
public String getDisplayName() { return "Remove multiple spaces in empty parentheses"; }
22+
public String getDisplayName() { return "Standardize spaces next to parentheses"; }
2423

2524
@Override
26-
public String getDescription() { return "Removes multiple spaces from empty parentheses (e.g. in method call chains)."; }
25+
public String getDescription() { return "Removes multiple spaces from empty parentheses and adds missing spaces between parentheses and character literals."; }
2726

2827
@Override
2928
public LocalDate getDateCreated() { return LocalDate.of(2021, 1, 3); }
@@ -37,23 +36,108 @@ public class SpacesInEmptyBracketsRule extends RuleForTokens {
3736
@Override
3837
public String getExample() {
3938
return ""
40-
+ LINE_SEP + " METHOD single_space_in_empty_parens."
41-
+ LINE_SEP + " ev_result = class_name( )=>get_tool( )->get_value( )."
39+
+ LINE_SEP + " METHOD standardize_spaces_in_parens."
40+
+ LINE_SEP + " ev_result = cl_any_factory=>get( )->get_utility( )->get_value( )."
41+
+ LINE_SEP
42+
+ LINE_SEP + " get_util( )->any_method( iv_any_param = get_default_value( )"
43+
+ LINE_SEP + " iv_other_param = VALUE #( ) )."
44+
+ LINE_SEP
45+
+ LINE_SEP + " \" these cases are syntactically correct even without spaces:"
46+
+ LINE_SEP + " any_method('text field literal')."
47+
+ LINE_SEP + " any_method('other literal' )."
48+
+ LINE_SEP + " any_method( 'third literal')."
49+
+ LINE_SEP
50+
+ LINE_SEP + " lt_any_table = VALUE #( ( '111')"
51+
+ LINE_SEP + " ( '222' )"
52+
+ LINE_SEP + " ( '333') )."
53+
+ LINE_SEP
54+
+ LINE_SEP + " lt_value = lt_other_table[ name = 'abc']-value."
55+
+ LINE_SEP
56+
+ LINE_SEP + " lt_amount = VALUE #( ( CONV #( '11.11') )"
57+
+ LINE_SEP + " ( CONV #('22.22' ) )"
58+
+ LINE_SEP + " ( CONV #( '33.33') ) )."
59+
+ LINE_SEP
60+
+ LINE_SEP + " lt_amount = VALUE #( ( CONV #('11.11') )"
61+
+ LINE_SEP + " ( CONV #('22.22') )"
62+
+ LINE_SEP + " ( CONV #('33.33') ) )."
63+
+ LINE_SEP
64+
+ LINE_SEP + " \" introducing spaces here would be a syntax error:"
65+
+ LINE_SEP + " CALL METHOD lo_instance->('METHOD_NAME')."
66+
+ LINE_SEP + " ls_struc-('COMPONENT_NAME') = 1."
67+
+ LINE_SEP + " lr_data_ref->('COMPONENT_NAME') = 1."
68+
+ LINE_SEP
69+
+ LINE_SEP + " \" the first two cases are executable but create a syntax warning without a space after the opening ("
70+
+ LINE_SEP + " other_method(`text string literal`)."
71+
+ LINE_SEP + " other_method(`other literal` )."
72+
+ LINE_SEP + " other_method( `third literal`)."
73+
+ LINE_SEP
74+
+ LINE_SEP + " lt_other_table = VALUE #( ( text = `abc` )"
75+
+ LINE_SEP + " ( text = `def`)"
76+
+ LINE_SEP + " ( text = `ghi`) )."
4277
+ LINE_SEP + " ENDMETHOD.";
4378
}
4479

80+
final ConfigBoolValue configRemoveMultiSpaceIfEmpty = new ConfigBoolValue(this, "RemoveMultiSpaceIfEmpty", "Remove multiple spaces from empty parentheses", true);
81+
final ConfigBoolValue configSeparateFromCharLiterals = new ConfigBoolValue(this, "SeparateFromCharLiterals", "Add space between parentheses and character literals", true, false, LocalDate.of(2023, 5, 30));
82+
final ConfigBoolValue configSeparateCondensedCases = new ConfigBoolValue(this, "SeparateCondensedCases", "Add space in condensed cases with single character literal: ...('...')", false, false, LocalDate.of(2023, 5, 30));
83+
84+
private final ConfigValue[] configValues = new ConfigValue[] { configRemoveMultiSpaceIfEmpty, configSeparateFromCharLiterals, configSeparateCondensedCases };
85+
86+
@Override
87+
public ConfigValue[] getConfigValues() { return configValues; }
88+
4589
public SpacesInEmptyBracketsRule(Profile profile) {
4690
super(profile);
4791
initializeConfiguration();
4892
}
4993

5094
@Override
5195
protected boolean executeOn(Code code, Command command, Token token, int releaseRestriction) {
52-
if (token.getOpensLevel() && !token.isLiteral() && !token.hasChildren() && token.getNext().closesLevel() && token.getNext().lineBreaks == 0 && token.getNext().spacesLeft > 1) {
53-
token.getNext().spacesLeft = 1;
54-
return true;
55-
} else {
96+
if (!token.getOpensLevel() || token.isLiteral())
5697
return false;
98+
99+
boolean changed = false;
100+
Token next = token.getNext();
101+
Token closingBracket = token.getNextSibling();
102+
if (token.hasChildren() && configSeparateFromCharLiterals.getValue()) {
103+
// exclude cases of ('...') where introducing spaces could be a syntax error, as in
104+
// - CALL METHOD ('METHOD_NAME'), CALL METHOD lo_instance->('METHOD_NAME'),
105+
// - ASSIGN ('(FUNC_GRP_NAME)TABLE[]') TO <lt_table>.
106+
// - ls_struc-('COMPONENT_NAME') = ..., lt_table[ 1 ]-('COMPONENT_NAME') = ...
107+
// - lr_data_ref->('COMPONENT_NAME') = ..., <ls_struc>-('COMPONENT_NAME') = ...
108+
// harmless cases such as CONV #('12.34') can be kept optionally (configKeepCondensedCases)
109+
if (next.isAttached() && next.getNext() == closingBracket && closingBracket.isAttached()) {
110+
if (!configSeparateCondensedCases.getValue() || token.textEqualsAny("(", "[") || token.textEndsWith("->(") || token.textEndsWith("=>(") || token.textEndsWith("-(")) {
111+
return false;
112+
}
113+
}
114+
115+
if (next.isCharacterLiteral() && next.isAttached()) {
116+
// add space between opening parenthesis and character literal: ('...' ) or (`...` )
117+
int startIndex = next.getStartIndexInLine();
118+
next.spacesLeft = 1;
119+
command.addIndent(1, startIndex, next, null, true);
120+
changed = true;
121+
} // do NOT attach the next section with 'else if'!
122+
123+
if (token.getLastChild().isCharacterLiteral() && closingBracket.isAttached()) {
124+
// add space between character literal and closing parenthesis: ( '...') or ( `...`)
125+
int startIndex = closingBracket.getStartIndexInLine();
126+
closingBracket.spacesLeft = 1;
127+
command.addIndent(1, startIndex, closingBracket, null, true);
128+
changed = true;
129+
}
130+
131+
} else if (!token.hasChildren() && configRemoveMultiSpaceIfEmpty.getValue()) {
132+
if (next == closingBracket && next.lineBreaks == 0 && next.spacesLeft > 1) {
133+
// remove multiple spaces in empty parentheses
134+
int startIndex = next.getStartIndexInLine();
135+
int addIndent = 1 - next.spacesLeft;
136+
next.spacesLeft = 1;
137+
command.addIndent(addIndent, startIndex, next, null, true);
138+
changed = true;
139+
}
57140
}
141+
return changed;
58142
}
59143
}

‎test/com.sap.adt.abapcleaner.test/src/com/sap/adt/abapcleaner/rules/spaces/SpacesInEmptyBracketsTest.java

+209
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,25 @@
11
package com.sap.adt.abapcleaner.rules.spaces;
22

3+
import org.junit.jupiter.api.BeforeEach;
34
import org.junit.jupiter.api.Test;
45

56
import com.sap.adt.abapcleaner.rulebase.RuleID;
67
import com.sap.adt.abapcleaner.rulebase.RuleTestBase;
78

89
class SpacesInEmptyBracketsTest extends RuleTestBase {
10+
private SpacesInEmptyBracketsRule rule;
11+
912
SpacesInEmptyBracketsTest() {
1013
super(RuleID.SPACES_IN_EMPTY_BRACKETS);
14+
rule = (SpacesInEmptyBracketsRule)getRule();
15+
}
16+
17+
@BeforeEach
18+
void setUp() {
19+
// setup default test configuration (may be modified in the individual test methods)
20+
rule.configRemoveMultiSpaceIfEmpty.setValue(true);
21+
rule.configSeparateFromCharLiterals.setValue(true);
22+
rule.configSeparateCondensedCases.setValue(false);
1123
}
1224

1325
@Test
@@ -21,6 +33,19 @@ void testMethodChain() {
2133
testRule();
2234
}
2335

36+
@Test
37+
void testMethodChainUnchanged() {
38+
rule.configRemoveMultiSpaceIfEmpty.setValue(false);
39+
40+
buildSrc(" ev_result = class_name( )=>get_tool( )->get_value( ).");
41+
42+
copyExpFromSrc();
43+
44+
putAnyMethodAroundSrcAndExp();
45+
46+
testRule();
47+
}
48+
2449
@Test
2550
void testCommentLineInsideCommand() {
2651
buildSrc(" class_name( )=>get_tool(");
@@ -49,4 +74,188 @@ void testLineBreaksNotRemoved() {
4974

5075
testRule();
5176
}
77+
78+
@Test
79+
void testTextFieldLiterals() {
80+
buildSrc(" any_method( 'text field literal' ).");
81+
buildSrc(" any_method('other literal' ).");
82+
buildSrc(" any_method( 'third literal').");
83+
buildSrc("");
84+
buildSrc(" lt_any_table = VALUE #( ( CONV #( '11.11') )");
85+
buildSrc(" ( CONV #('22.22' ) )");
86+
buildSrc(" ( CONV #( '33.33') ) ).");
87+
88+
buildExp(" any_method( 'text field literal' ).");
89+
buildExp(" any_method( 'other literal' ).");
90+
buildExp(" any_method( 'third literal' ).");
91+
buildExp("");
92+
buildExp(" lt_any_table = VALUE #( ( CONV #( '11.11' ) )");
93+
buildExp(" ( CONV #( '22.22' ) )");
94+
buildExp(" ( CONV #( '33.33' ) ) ).");
95+
96+
putAnyMethodAroundSrcAndExp();
97+
98+
testRule();
99+
}
100+
101+
@Test
102+
void testTextFieldLiteralsUnchanged() {
103+
rule.configSeparateFromCharLiterals.setValue(false);
104+
105+
buildSrc(" any_method( 'text field literal' ).");
106+
buildSrc(" any_method('other literal' ).");
107+
buildSrc(" any_method( 'third literal').");
108+
buildSrc("");
109+
buildSrc(" lt_any_table = VALUE #( ( CONV #( '11.11') )");
110+
buildSrc(" ( CONV #('22.22' ) )");
111+
buildSrc(" ( CONV #('33.33') ) ).");
112+
113+
copyExpFromSrc();
114+
115+
putAnyMethodAroundSrcAndExp();
116+
117+
testRule();
118+
}
119+
120+
@Test
121+
void testDynamicMethodCallUnchanged() {
122+
// introducing spaces in the following dynamic examples would be a syntax error
123+
buildSrc(" CALL METHOD ('METHOD_NAME').");
124+
buildSrc(" CALL METHOD lo_instance->('METHOD_NAME').");
125+
buildSrc("");
126+
buildSrc(" CALL METHOD (`METHOD_NAME`).");
127+
buildSrc(" CALL METHOD lo_instance->(`METHOD_NAME`).");
128+
buildSrc("");
129+
buildSrc(" ASSIGN ('(FUNC_GRP_NAME)TABLE[]') TO <lt_table>.");
130+
buildSrc("");
131+
buildSrc(" ls_struc-('COMPONENT_NAME') = 1.");
132+
buildSrc(" <ls_struc>-('COMPONENT_NAME') = 1.");
133+
buildSrc(" lt_table[ 1 ]-('COMPONENT_NAME') = 1.");
134+
buildSrc(" lr_data_ref->('COMPONENT_NAME') = 1.");
135+
136+
copyExpFromSrc();
137+
138+
putAnyMethodAroundSrcAndExp();
139+
140+
testRule();
141+
}
142+
143+
@Test
144+
void testTextStringLiterals() {
145+
buildSrc(" other_method( `text string literal` ).");
146+
buildSrc(" other_method(`other literal` ).");
147+
buildSrc(" other_method( `third literal`).");
148+
buildSrc("");
149+
buildSrc(" lt_other_table = VALUE #( ( `abc` )");
150+
buildSrc(" ( `def`)");
151+
buildSrc(" ( `ghi`) ).");
152+
153+
buildExp(" other_method( `text string literal` ).");
154+
buildExp(" other_method( `other literal` ).");
155+
buildExp(" other_method( `third literal` ).");
156+
buildExp("");
157+
buildExp(" lt_other_table = VALUE #( ( `abc` )");
158+
buildExp(" ( `def` )");
159+
buildExp(" ( `ghi` ) ).");
160+
161+
putAnyMethodAroundSrcAndExp();
162+
163+
testRule();
164+
}
165+
166+
@Test
167+
void testTextStringLiteralsUnchanged() {
168+
rule.configSeparateFromCharLiterals.setValue(false);
169+
170+
buildSrc(" other_method( `text string literal` ).");
171+
buildSrc(" other_method(`other literal` ).");
172+
buildSrc(" other_method( `third literal`).");
173+
buildSrc("");
174+
buildSrc(" lt_other_table = VALUE #( ( `abc` )");
175+
buildSrc(" ( `def`)");
176+
buildSrc(" ( `ghi`) ).");
177+
178+
copyExpFromSrc();
179+
180+
putAnyMethodAroundSrcAndExp();
181+
182+
testRule();
183+
}
184+
185+
@Test
186+
void testIndentAdjustedForEmptyParens() {
187+
// expect the indent of the second and third line to be adjusted, but not the indent of the last three lines
188+
buildSrc(" cl_any_factory=>get( )->get_utility( )->any_method( iv_any_param = 1");
189+
buildSrc(" iv_other_param = 2");
190+
buildSrc(" iv_third_param = 3 ).");
191+
192+
buildExp(" cl_any_factory=>get( )->get_utility( )->any_method( iv_any_param = 1");
193+
buildExp(" iv_other_param = 2");
194+
buildExp(" iv_third_param = 3 ).");
195+
196+
testRule();
197+
}
198+
199+
@Test
200+
void testIndentAdjustedForAttachedLiterals() {
201+
// expect the indent of the second and third line to be adjusted, but not the indent of the last three lines
202+
buildSrc(" cl_any_factory=>get( )->get_utility( )->any_method( iv_any_param = 1");
203+
buildSrc(" iv_other_param = 2");
204+
buildSrc(" iv_third_param = 3 ).");
205+
buildSrc("");
206+
buildSrc(" cl_any_factory=>get( )->get_utility( )->any_method(");
207+
buildSrc(" iv_any_param = 1");
208+
buildSrc(" iv_other_param = 2");
209+
buildSrc(" iv_third_param = 3 ).");
210+
211+
buildExp(" cl_any_factory=>get( )->get_utility( )->any_method( iv_any_param = 1");
212+
buildExp(" iv_other_param = 2");
213+
buildExp(" iv_third_param = 3 ).");
214+
buildExp("");
215+
buildExp(" cl_any_factory=>get( )->get_utility( )->any_method(");
216+
buildExp(" iv_any_param = 1");
217+
buildExp(" iv_other_param = 2");
218+
buildExp(" iv_third_param = 3 ).");
219+
220+
testRule();
221+
}
222+
223+
@Test
224+
void testKeepCondensedCases() {
225+
buildSrc(" any_method('text field literal').");
226+
buildSrc(" other_method(`text string literal`).");
227+
buildSrc("");
228+
buildSrc(" lt_amount = VALUE #( ( CONV #('11.11') )");
229+
buildSrc(" ( CONV #('22.22') )");
230+
buildSrc(" ( CONV #('33.33') ) ).");
231+
232+
copyExpFromSrc();
233+
234+
putAnyMethodAroundSrcAndExp();
235+
236+
testRule();
237+
}
238+
239+
@Test
240+
void testChangeCondensedCases() {
241+
rule.configSeparateCondensedCases.setValue(true);
242+
243+
buildSrc(" any_method('text field literal').");
244+
buildSrc(" other_method(`text string literal`).");
245+
buildSrc("");
246+
buildSrc(" lt_amount = VALUE #( ( CONV #('11.11') )");
247+
buildSrc(" ( CONV #('22.22') )");
248+
buildSrc(" ( CONV #('33.33') ) ).");
249+
250+
buildExp(" any_method( 'text field literal' ).");
251+
buildExp(" other_method( `text string literal` ).");
252+
buildExp("");
253+
buildExp(" lt_amount = VALUE #( ( CONV #( '11.11' ) )");
254+
buildExp(" ( CONV #( '22.22' ) )");
255+
buildExp(" ( CONV #( '33.33' ) ) ).");
256+
257+
putAnyMethodAroundSrcAndExp();
258+
259+
testRule();
260+
}
52261
}

0 commit comments

Comments
 (0)