Skip to content

Commit 5d9babe

Browse files
GCI100 StringConcatenation fix: resolve conflicts
Co-authored-by: DataLabGroupe-CreditAgricole <[email protected]>
2 parents db77a56 + e9c432b commit 5d9babe

File tree

8 files changed

+238
-17
lines changed

8 files changed

+238
-17
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010
### Added
1111

1212
- [#73](https://github.com/green-code-initiative/creedengo-python/pull/73) Add rule GCI100 Disable Gradient For model eval, a rule specific to PyTorch and AI/ML
13+
- [#78](https://github.com/green-code-initiative/creedengo-python/pull/78) Add rule GCI105 on String Concatenation. This rule may also apply to other rules
1314
- [#74](https://github.com/green-code-initiative/creedengo-python/pull/74) Add rule GCI101 Avoid Conv Bias Before Batch Normalization, a rule specific to Deeplearning
1415
- [#75](https://github.com/green-code-initiative/creedengo-python/pull/75) Add rule GCI102 avoid non pinned memory for dataloader. This rule is specific to PyTorch and so AI
1516
- [#68](https://github.com/green-code-initiative/creedengo-python/pull/68) Data : Add rule GCI107 Avoid Iterative Matrix Operations. Use vectorization by the usage of the built-in functions of TensorFlow, NumPy or Pandas

src/it/java/org/greencodeinitiative/creedengo/python/integration/tests/GCIRulesIT.java

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -320,21 +320,6 @@ void testGCI99(){
320320

321321
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_50MIN);
322322
}
323-
324-
@Test
325-
void testGCI102(){
326-
String filePath = "src/avoidNonPinnedMemoryForDataloaders.py";
327-
String ruleId = "creedengo-python:GCI102";
328-
String ruleMsg = "Use pinned memory to reduce data transfer in RAM.";
329-
int[] startLines = new int[]{
330-
7, 8, 9, 10, 11, 12, 13, 14
331-
};
332-
int[] endLines = new int[]{
333-
7, 8, 9, 10, 11, 12, 13, 14
334-
};
335-
336-
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_10MIN);
337-
}
338323

339324
@Test
340325
void testGCI100() {
@@ -366,7 +351,22 @@ void testGCI101(){
366351

367352
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_10MIN);
368353
}
369-
354+
355+
@Test
356+
void testGCI102(){
357+
String filePath = "src/avoidNonPinnedMemoryForDataloaders.py";
358+
String ruleId = "creedengo-python:GCI102";
359+
String ruleMsg = "Use pinned memory to reduce data transfer in RAM.";
360+
int[] startLines = new int[]{
361+
7, 8, 9, 10, 11, 12, 13, 14
362+
};
363+
int[] endLines = new int[]{
364+
7, 8, 9, 10, 11, 12, 13, 14
365+
};
366+
367+
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_10MIN);
368+
}
369+
370370
@Test
371371
void testGCI103(){
372372

@@ -380,6 +380,22 @@ void testGCI103(){
380380
5, 8, 24, 27, 36
381381
};
382382

383+
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_1MIN);
384+
}
385+
386+
@Test
387+
void testGCI105() {
388+
389+
String filePath = "src/stringConcatenation.py";
390+
String ruleId = "creedengo-python:GCI105";
391+
String ruleMsg = "Concatenation of strings should be done using f-strings or str.join()";
392+
int[] startLines = new int[]{
393+
5, 8, 10, 32, 38
394+
};
395+
int[] endLines = new int[]{
396+
5, 8, 10, 32, 38
397+
};
398+
383399
checkIssuesForFile(filePath, ruleId, ruleMsg, startLines, endLines, SEVERITY, TYPE, EFFORT_1MIN);
384400
}
385401

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
text = "hello"
2+
following_words = ["world", "I", "am", "a", "string", "concatenation"]
3+
4+
for word in following_words:
5+
text += word # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
6+
7+
text = "init"
8+
text += " add this" # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
9+
10+
text += [word for word in following_words] # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
11+
12+
13+
result = " ".join([text] + following_words)
14+
15+
16+
final = f"{text} {' '.join(following_words)}"
17+
18+
19+
def build_string(base, parts):
20+
return f"{base} {' '.join(parts)}"
21+
22+
mylist = []
23+
mylist += [1, 2, 3] # Compliant
24+
25+
26+
count = 0
27+
count += 1 # Compliant
28+
29+
30+
msg = "start"
31+
if True:
32+
msg += " continued" # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
33+
34+
35+
def get_text():
36+
return "function text"
37+
38+
text += get_text() # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}

src/main/java/org/greencodeinitiative/creedengo/python/PythonRuleRepository.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,8 @@ public class PythonRuleRepository implements RulesDefinition, PythonCustomRuleRe
4949
AvoidIterativeMatrixOperations.class,
5050
AvoidNonPinnedMemoryForDataloaders.class,
5151
AvoidConvBiasBeforeBatchNorm.class,
52-
DisableGradientForModelEval.class
52+
DisableGradientForModelEval.class,
53+
StringConcatenation.class
5354
);
5455

5556
public static final String LANGUAGE = "py";
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs
3+
* Copyright © 2024 Green Code Initiative (https://green-code-initiative.org)
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
package org.greencodeinitiative.creedengo.python.checks;
19+
20+
import org.sonar.check.Rule;
21+
import org.sonar.plugins.python.api.PythonSubscriptionCheck;
22+
import org.sonar.plugins.python.api.SubscriptionContext;
23+
import org.sonar.plugins.python.api.tree.Name;
24+
import org.sonar.plugins.python.api.tree.CompoundAssignmentStatement;
25+
import org.sonar.plugins.python.api.tree.Tree;
26+
import org.sonar.plugins.python.api.tree.Expression;
27+
import org.sonar.plugins.python.api.tree.AssignmentStatement;
28+
29+
import java.util.ArrayList;
30+
import java.util.List;
31+
32+
@Rule(key = "GCI105")
33+
public class StringConcatenation extends PythonSubscriptionCheck {
34+
35+
private final List<String> stringVariables = new ArrayList<>();
36+
37+
public static final String DESCRIPTION = "Concatenation of strings should be done using f-strings or str.join()";
38+
39+
@Override
40+
public void initialize(Context context) {
41+
context.registerSyntaxNodeConsumer(Tree.Kind.ASSIGNMENT_STMT, this::trackVariableAssignments);
42+
context.registerSyntaxNodeConsumer(Tree.Kind.COMPOUND_ASSIGNMENT, this::checkAssignment);
43+
}
44+
45+
private void checkAssignment(SubscriptionContext context) {
46+
CompoundAssignmentStatement compoundAssignment = (CompoundAssignmentStatement) context.syntaxNode();
47+
if ("+=".equals(compoundAssignment.compoundAssignmentToken().value())) {
48+
Expression lhsExpression = compoundAssignment.lhsExpression();
49+
if (lhsExpression.is(Tree.Kind.NAME)) {
50+
String variableName = ((Name) lhsExpression).name();
51+
if (stringVariables.contains(variableName)) {
52+
context.addIssue(lhsExpression.firstToken(), DESCRIPTION);
53+
}
54+
}
55+
}
56+
}
57+
58+
private void trackVariableAssignments(SubscriptionContext context) {
59+
AssignmentStatement assignment = (AssignmentStatement) context.syntaxNode();
60+
if (assignment.lhsExpressions().size() == 1) {
61+
Expression lhs = assignment.lhsExpressions().get(0).expressions().get(0);
62+
if (lhs.is(Tree.Kind.NAME)) {
63+
String variableName = ((Name) lhs).name();
64+
Expression assignedValue = assignment.assignedValue();
65+
if (isStringAssignment(assignedValue)) {
66+
if (!stringVariables.contains(variableName)) {
67+
stringVariables.add(variableName);
68+
}
69+
} else {
70+
stringVariables.remove(variableName);
71+
}
72+
}
73+
}
74+
}
75+
76+
private boolean isStringAssignment(Expression assignedValue) {
77+
return assignedValue.is(Tree.Kind.STRING_ELEMENT) ||
78+
assignedValue.is(Tree.Kind.STRING_LITERAL) ||
79+
containsStringElement(assignedValue);
80+
}
81+
82+
private boolean containsStringElement(Tree node) {
83+
if (node.is(Tree.Kind.STRING_ELEMENT) || node.is(Tree.Kind.STRING_LITERAL)) {
84+
return true;
85+
}
86+
for (Tree child : node.children()) {
87+
if (containsStringElement(child)) {
88+
return true;
89+
}
90+
}
91+
return false;
92+
}
93+
94+
}

src/main/resources/org/greencodeinitiative/creedengo/python/creedengo_way_profile.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
"GCI101",
1818
"GCI102",
1919
"GCI103",
20+
"GCI105",
2021
"GCI106",
2122
"GCI107",
2223
"GCI203",
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/*
2+
* creedengo - Python language - Provides rules to reduce the environmental footprint of your Python programs
3+
* Copyright © 2024 Green Code Initiative (https://green-code-initiative.org)
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
package org.greencodeinitiative.creedengo.python.checks;
19+
20+
import org.junit.Test;
21+
import org.sonar.python.checks.utils.PythonCheckVerifier;
22+
23+
public class StringConcatenationTest {
24+
25+
@Test
26+
public void test() {
27+
PythonCheckVerifier.verify("src/test/resources/checks/stringConcatenation.py", new StringConcatenation());
28+
}
29+
}
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
text = "hello"
2+
following_words = ["world", "I", "am", "a", "string", "concatenation"]
3+
4+
for word in following_words:
5+
text += word # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
6+
7+
text = "init"
8+
text += " add this" # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
9+
10+
text += [word for word in following_words] # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
11+
12+
text = 0
13+
text +=1
14+
15+
text = "start"
16+
result = " ".join([text] + following_words)
17+
18+
19+
final = f"{text} {' '.join(following_words)}"
20+
21+
22+
def build_string(base, parts):
23+
return f"{base} {' '.join(parts)}"
24+
25+
mylist = []
26+
mylist += [1, 2, 3] # Compliant
27+
28+
29+
count = 0
30+
count += 1 # Compliant
31+
32+
33+
msg = "start"
34+
if True:
35+
msg += " continued" # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}
36+
37+
38+
def get_text():
39+
return "function text"
40+
41+
text += get_text() # Noncompliant {{Concatenation of strings should be done using f-strings or str.join()}}

0 commit comments

Comments
 (0)