Skip to content

Commit 057d9b4

Browse files
jgardn3rfourls
jgardn3r
authored and
fourls
committed
Fix token index OOB exceptions in RedundantInherited 
1 parent a493af6 commit 057d9b4

File tree

4 files changed

+182
-30
lines changed

4 files changed

+182
-30
lines changed

‎delphi-checks/src/main/java/au/com/integradev/delphi/checks/RedundantInheritedCheck.java

Lines changed: 41 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Collections;
2323
import java.util.List;
2424
import java.util.stream.Collectors;
25+
import java.util.stream.IntStream;
2526
import org.sonar.check.Rule;
2627
import org.sonar.plugins.communitydelphi.api.ast.CompoundStatementNode;
2728
import org.sonar.plugins.communitydelphi.api.ast.DelphiNode;
@@ -59,45 +60,34 @@ public DelphiCheckContext visit(RoutineImplementationNode routine, DelphiCheckCo
5960

6061
private static List<QuickFix> getQuickFixes(
6162
DelphiNode violationNode, DelphiCheckContext context) {
62-
DelphiNode statement = violationNode;
63+
DelphiNode violationStatement = violationNode;
6364
DelphiNode parent = violationNode.getParent();
6465
while (!(parent instanceof StatementListNode)) {
65-
statement = parent;
66+
violationStatement = parent;
6667
parent = parent.getParent();
6768
if (parent == null) {
6869
return Collections.emptyList();
6970
}
7071
}
7172
StatementListNode statementListNode = (StatementListNode) parent;
72-
int statementIndex = statementListNode.getStatements().indexOf(statement);
73+
int statementIndex = statementListNode.getStatements().indexOf(violationStatement);
7374

74-
int startLine = statement.getBeginLine();
75-
int startCol = statement.getBeginColumn();
76-
int endLine = statement.getEndLine();
77-
int endCol = statement.getEndColumn();
75+
int startLine = violationStatement.getBeginLine();
76+
int startCol = violationStatement.getBeginColumn();
77+
78+
DelphiToken lastStatementToken = getLastStatementToken(statementListNode, violationStatement);
79+
int endLine = lastStatementToken.getEndLine();
80+
int endCol = lastStatementToken.getEndColumn();
7881

7982
if (statementListNode.getStatements().size() > statementIndex + 1) {
80-
int tokenIndex = statement.getLastToken().getIndex() + 1;
81-
final List<DelphiTokenType> skipTokenTypes =
82-
List.of(DelphiTokenType.SEMICOLON, DelphiTokenType.WHITESPACE);
83-
while (skipTokenTypes.contains(context.getTokens().get(tokenIndex).getType())) {
84-
tokenIndex++;
85-
}
86-
DelphiToken statementNode = context.getTokens().get(tokenIndex);
87-
endLine = statementNode.getBeginLine();
88-
endCol = statementNode.getBeginColumn();
83+
DelphiToken lastDeletableToken = nextNonWhitespaceToken(context, lastStatementToken, true);
84+
endLine = lastDeletableToken.getBeginLine();
85+
endCol = lastDeletableToken.getBeginColumn();
8986
} else if (statementIndex > 0) {
90-
int tokenIndex = statement.getLastToken().getIndex() - 1;
91-
while (context.getTokens().get(tokenIndex).getType() == DelphiTokenType.WHITESPACE) {
92-
tokenIndex--;
93-
}
94-
DelphiToken statementNode = context.getTokens().get(tokenIndex);
95-
startLine = statementNode.getEndLine();
96-
startCol = statementNode.getEndColumn();
97-
} else {
98-
DelphiNode lastToken = getLastStatementToken(statementListNode, statement);
99-
endLine = lastToken.getEndLine();
100-
endCol = lastToken.getEndColumn();
87+
DelphiToken lastDeletableToken =
88+
nextNonWhitespaceToken(context, violationStatement.getFirstToken(), false);
89+
startLine = lastDeletableToken.getEndLine();
90+
startCol = lastDeletableToken.getEndColumn();
10191
}
10292

10393
return List.of(
@@ -106,17 +96,39 @@ private static List<QuickFix> getQuickFixes(
10696
QuickFixEdit.delete(FilePosition.from(startLine, startCol, endLine, endCol))));
10797
}
10898

99+
private static DelphiToken nextNonWhitespaceToken(
100+
DelphiCheckContext context, DelphiToken startingToken, boolean forward) {
101+
int step = forward ? 1 : -1;
102+
int contextIndex = getContextIndex(context, startingToken);
103+
if (contextIndex == -1) {
104+
return startingToken;
105+
}
106+
107+
do {
108+
contextIndex += step;
109+
} while (context.getTokens().get(contextIndex).getType() == DelphiTokenType.WHITESPACE);
110+
return context.getTokens().get(contextIndex);
111+
}
112+
109113
private static boolean isSemicolon(DelphiNode node) {
110114
return node.getChildren().isEmpty() && node.getToken().getType() == DelphiTokenType.SEMICOLON;
111115
}
112116

113-
private static DelphiNode getLastStatementToken(
117+
private static int getContextIndex(DelphiCheckContext context, DelphiToken token) {
118+
return IntStream.range(0, context.getTokens().size())
119+
.filter(tokenIndex -> context.getTokens().get(tokenIndex).getIndex() == token.getIndex())
120+
.findFirst()
121+
.orElse(-1);
122+
}
123+
124+
private static DelphiToken getLastStatementToken(
114125
StatementListNode statementListNode, DelphiNode node) {
115126
return statementListNode.getChildren().stream()
116127
.skip(node.getChildIndex() + 1L)
117128
.takeWhile(RedundantInheritedCheck::isSemicolon)
118129
.reduce((first, second) -> second)
119-
.orElse(node);
130+
.orElse(node)
131+
.getLastToken();
120132
}
121133

122134
private static List<DelphiNode> findViolations(RoutineImplementationNode routine) {

‎delphi-checks/src/test/java/au/com/integradev/delphi/checks/RedundantInheritedCheckTest.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919
package au.com.integradev.delphi.checks;
2020

21+
import au.com.integradev.delphi.builders.DelphiTestFile;
2122
import au.com.integradev.delphi.builders.DelphiTestUnitBuilder;
2223
import au.com.integradev.delphi.checks.verifier.CheckVerifier;
2324
import org.junit.jupiter.api.Test;
@@ -203,9 +204,19 @@ void testNotOverridingMethodShouldAddIssues() {
203204
.appendImpl(" // Fix qf3@[+1:4 to +1:14] <<>>")
204205
.appendImpl(" inherited; // Noncompliant")
205206
.appendImpl(" end;")
206-
.appendImpl(" // Fix qf4@[+0:34 to +1:11] <<>>")
207+
.appendImpl(" // Fix qf4@[+0:34 to +1:12] <<>>")
207208
.appendImpl(" inherited; // Noncompliant")
208209
.appendImpl("end;"))
209210
.verifyIssues();
210211
}
212+
213+
@Test
214+
void testQuickFixesFollowingIncludeDirective() {
215+
CheckVerifier.newVerifier()
216+
.withCheck(new RedundantInheritedCheck())
217+
.onFile(
218+
DelphiTestFile.fromResource(
219+
"/au/com/integradev/delphi/checks/RedundantInherited/QuickFixesFollowingIncludeDirectives.pas"))
220+
.verifyIssues();
221+
}
211222
}

‎delphi-checks/src/test/resources/au/com/integradev/delphi/checks/RedundantInherited/QuickFixesFollowingIncludeDirectives.inc

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// This is just to offset the token indices
2+
{$ifdef A}{$endif}
3+
{$ifdef A}{$endif}
4+
{$ifdef A}{$endif}
5+
{$ifdef A}{$endif}
6+
{$ifdef A}{$endif}
7+
{$ifdef A}{$endif}
8+
{$ifdef A}{$endif}
9+
{$ifdef A}{$endif}
10+
{$ifdef A}{$endif}
11+
{$ifdef A}{$endif}
12+
{$ifdef A}{$endif}
13+
{$ifdef A}{$endif}
14+
{$ifdef A}{$endif}
15+
{$ifdef A}{$endif}
16+
{$ifdef A}{$endif}
17+
{$ifdef A}{$endif}
18+
{$ifdef A}{$endif}
19+
{$ifdef A}{$endif}
20+
{$ifdef A}{$endif}
21+
{$ifdef A}{$endif}
22+
{$ifdef A}{$endif}
23+
{$ifdef A}{$endif}
24+
{$ifdef A}{$endif}
25+
{$ifdef A}{$endif}
26+
{$ifdef A}{$endif}
27+
{$ifdef A}{$endif}
28+
{$ifdef A}{$endif}
29+
{$ifdef A}{$endif}
30+
{$ifdef A}{$endif}
31+
{$ifdef A}{$endif}
32+
{$ifdef A}{$endif}
33+
{$ifdef A}{$endif}
34+
{$ifdef A}{$endif}
35+
{$ifdef A}{$endif}
36+
{$ifdef A}{$endif}
37+
{$ifdef A}{$endif}
38+
{$ifdef A}{$endif}
39+
{$ifdef A}{$endif}
40+
{$ifdef A}{$endif}
41+
{$ifdef A}{$endif}
42+
{$ifdef A}{$endif}
43+
{$ifdef A}{$endif}
44+
{$ifdef A}{$endif}
45+
{$ifdef A}{$endif}
46+
{$ifdef A}{$endif}
47+
{$ifdef A}{$endif}
48+
{$ifdef A}{$endif}
49+
{$ifdef A}{$endif}
50+
{$ifdef A}{$endif}
51+
{$ifdef A}{$endif}
52+
{$ifdef A}{$endif}
53+
{$ifdef A}{$endif}
54+
{$ifdef A}{$endif}
55+
{$ifdef A}{$endif}
56+
{$ifdef A}{$endif}
57+
{$ifdef A}{$endif}
58+
{$ifdef A}{$endif}
59+
{$ifdef A}{$endif}
60+
{$ifdef A}{$endif}
61+
{$ifdef A}{$endif}
62+
{$ifdef A}{$endif}
63+
{$ifdef A}{$endif}
64+
{$ifdef A}{$endif}
65+
{$ifdef A}{$endif}
66+
{$ifdef A}{$endif}
67+
{$ifdef A}{$endif}
68+
{$ifdef A}{$endif}
69+
{$ifdef A}{$endif}
70+
{$ifdef A}{$endif}
71+
{$ifdef A}{$endif}
72+
{$ifdef A}{$endif}
73+
{$ifdef A}{$endif}
74+
{$ifdef A}{$endif}
75+
{$ifdef A}{$endif}
76+
{$ifdef A}{$endif}
77+
{$ifdef A}{$endif}
78+
{$ifdef A}{$endif}
79+
{$ifdef A}{$endif}
80+
{$ifdef A}{$endif}
81+
{$ifdef A}{$endif}
82+
{$ifdef A}{$endif}
83+
{$ifdef A}{$endif}
84+
{$ifdef A}{$endif}
85+
{$ifdef A}{$endif}
86+
{$ifdef A}{$endif}
87+
{$ifdef A}{$endif}
88+
{$ifdef A}{$endif}
89+
{$ifdef A}{$endif}
90+
{$ifdef A}{$endif}
91+
{$ifdef A}{$endif}
92+
{$ifdef A}{$endif}
93+
{$ifdef A}{$endif}
94+
{$ifdef A}{$endif}
95+
{$ifdef A}{$endif}
96+
{$ifdef A}{$endif}
97+
{$ifdef A}{$endif}
98+
{$ifdef A}{$endif}
99+
{$ifdef A}{$endif}
100+
{$ifdef A}{$endif}
101+
{$ifdef A}{$endif}

‎delphi-checks/src/test/resources/au/com/integradev/delphi/checks/RedundantInherited/QuickFixesFollowingIncludeDirectives.pas

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
unit Test;
2+
{$I QuickFixesFollowingIncludeDirectives.inc}
3+
interface
4+
type
5+
TFoo = class(TObject)
6+
procedure Foo;
7+
procedure Bar;
8+
end;
9+
10+
implementation
11+
12+
procedure TFoo.Foo;
13+
begin
14+
// Fix qf1@[+2:2 to +3:2] <<>>
15+
// Noncompliant@+1
16+
inherited;
17+
B := 1;
18+
end;
19+
20+
procedure TFoo.Bar;
21+
begin
22+
var B := 1;
23+
inherited;
24+
// Fix qf2@[-2:13 to -1:12] <<>>
25+
// Noncompliant@-2
26+
end;
27+
28+
end.

0 commit comments

Comments
 (0)