Use "slices.Equal()" instead of custom implementation of this function by paulnegz · Pull Request #100 · SonarSource/sonar-go

paulnegz · 2025-07-30T16:55:17Z

This commit introduces a new check to detect manually implemented slice equality functions and suggest using the standard slices.Equal() function instead.

The SliceEqualSimplificationCheck identifies functions that have a specific structure:

Two slice parameters
A length comparison
A for-loop for element-wise comparison
A final return true statement

This helps improve code consistency and maintainability by encouraging the use of the standard library. Comprehensive tests have been added to cover various positive and negative cases to ensure the check is accurate.

Part of

This commit introduces a new check to detect manually implemented slice equality functions and suggest using the standard `slices.Equal()` function instead. The `SliceEqualSimplificationCheck` identifies functions that have a specific structure: - Two slice parameters - A length comparison - A for-loop for element-wise comparison - A final `return true` statement This helps improve code consistency and maintainability by encouraging the use of the standard library. Comprehensive tests have been added to cover various positive and negative cases to ensure the check is accurate.

mstachniuk · 2025-09-03T12:16:12Z

...g/slang-checks/src/main/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheck.java

@@ -0,0 +1,192 @@
+/*


Please move this file to: sonar-go-checks/src/main/java/org/sonar/go/checks/SliceEqualSimplificationCheck.java

mstachniuk · 2025-09-03T12:16:47Z

...ang-checks/src/test/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheckTest.java

@@ -0,0 +1,27 @@
+/*


Please move this file to: sonar-go-checks/src/test/java/org/sonar/go/checks/SliceEqualSimplificationCheckTest.java

mstachniuk · 2025-09-03T12:18:17Z

.../slang-checks/src/test/resources/org/sonarsource/slang/checks/SliceEqualSimplification.slang

@@ -0,0 +1,100 @@
+// Positive cases


Please move this file to sonar-go-checks/src/test/resources/checks/SliceEqualSimplification/SliceEqualSimplification.go
(There is go extension instead of slang). Now in sonar-go we are not using *.slang files anymore but Go code.

Please add package SliceEqualSimplification at the beginning of the file. Otherwise, the file will be not parsed.

Please use Sensitive cases instead of Positive cases

mstachniuk · 2025-09-03T12:18:53Z

...g/slang-checks/src/main/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheck.java

+import org.sonarsource.slang.checks.api.SlangCheck;
+
+@Rule(key = "S-slice-equal")
+public class SliceEqualSimplificationCheck implements SlangCheck {


Suggested change

public class SliceEqualSimplificationCheck implements SlangCheck {

public class SliceEqualSimplificationCheck implements GoCheck {

mstachniuk · 2025-09-03T12:25:02Z

...g/slang-checks/src/main/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheck.java

+    // Semantic checks would be needed here to verify types.
+    // For example:
+    // if (!isSlice(param1.type()) || !isSlice(param2.type())) {
+    //   return false;
+    // }
+    // if (!areElementTypesComparableAndIdentical(param1.type(), param2.type())) {
+    //   return false;
+    // }


For now it can be approximated with:

private static boolean areParametersSlices(ParameterTree param1, ParameterTree param2) { return param1.type().type().equals(param2.type().type()); }

However it requires more complex logic, to cover some cases:

func equalSlicesWithDifferentVarNames(slice1, slice2 []int) bool - first type is skipped

func Equal[S ~[]E, E comparable](s1, s2 S) bool - original signature from Go standard library

func (rec *MyStruct) equalSlicesReceiver(slice1, slice2 []int) bool - what if there is receiver?

func equalSlicesFirstParamAsPointer(s1 *[]int, s2 []int) bool - what if there is a pointer?

maybe some other possible cases?

Please try to cover them.

mstachniuk · 2025-09-03T12:44:19Z

...g/slang-checks/src/main/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheck.java

+  private static boolean isReturnFalse(Tree statement) {
+    if (!(statement instanceof ReturnTree)) {
+      return false;
+    }
+    ReturnTree returnTree = (ReturnTree) statement;
+    Tree body = returnTree.body();
+    return body instanceof IdentifierTree && "false".equals(((IdentifierTree) body).name());
+  }
+
+  private static boolean isReturnTrue(Tree statement) {
+    if (!(statement instanceof ReturnTree)) {
+      return false;
+    }
+    ReturnTree returnTree = (ReturnTree) statement;
+    Tree body = returnTree.body();
+    return body instanceof IdentifierTree && "true".equals(((IdentifierTree) body).name());
+  }


Suggested change

private static boolean isReturnFalse(Tree statement) {

if (!(statement instanceof ReturnTree)) {

return false;

}

ReturnTree returnTree = (ReturnTree) statement;

Tree body = returnTree.body();

return body instanceof IdentifierTree && "false".equals(((IdentifierTree) body).name());

}

private static boolean isReturnTrue(Tree statement) {

if (!(statement instanceof ReturnTree)) {

return false;

}

ReturnTree returnTree = (ReturnTree) statement;

Tree body = returnTree.body();

return body instanceof IdentifierTree && "true".equals(((IdentifierTree) body).name());

}

private static boolean isReturnFalse(Tree statement) {

return isReturn(statement, "false");

}

private static boolean isReturnTrue(Tree statement) {

return isReturn(statement, "true");

}

private static boolean isReturn(Tree statement, String returnValue) {

if (statement instanceof ReturnTree returnTree &&

returnTree.expressions().size() == 1) {

Tree body = returnTree.expressions().get(0);

return body instanceof LiteralTree LiteralTreeBody && returnValue.equals(LiteralTreeBody.value());

}

return false;

}

mstachniuk · 2025-09-03T12:44:50Z

...ang-checks/src/test/java/org/sonarsource/slang/checks/SliceEqualSimplificationCheckTest.java

+
+  @Test
+  void test() {
+    Verifier.verify("SliceEqualSimplification.slang", new SliceEqualSimplificationCheck());


Suggested change

Verifier.verify("SliceEqualSimplification.slang", new SliceEqualSimplificationCheck());

GoVerifier.verify("SliceEqualSimplification/SliceEqualSimplification.go", new SliceEqualSimplificationCheck());

mstachniuk · 2025-09-03T12:49:04Z

.../slang-checks/src/test/resources/org/sonarsource/slang/checks/SliceEqualSimplification.slang

@@ -0,0 +1,100 @@
+// Positive cases
+fun equalSlices(a, b) { // Noncompliant {{Use slices.Equal() instead of custom slice comparison}}


Please add:

// ^^^^^^^^^^^

in next line, to indicate that the issue should be only raised on the method name.

mstachniuk · 2025-09-03T12:50:51Z

.../slang-checks/src/test/resources/org/sonarsource/slang/checks/SliceEqualSimplification.slang

+  }
+  return true
+}
+


Please add following cases:

// function with generics like in slices.Equal() func Equal[S ~[]E, E comparable](s1, s2 S) bool { // Noncompliant {{Use slices.Equal() instead of custom slice comparison}} if len(s1) != len(s2) { return false } for i := range s1 { if s1[i] != s2[i] { return false } } return true } func (rec *MyStruct) equalSlicesReceiver(slice1, slice2 []int) bool { // Noncompliant {{Use slices.Equal() instead of custom slice comparison}} if len(slice1) != len(slice2) { return false } for i := range slice1 { if slice1[i] != slice2[i] { return false } } return true } func equalSlicesFirstParamAsPointer(s1 *[]int, s2 []int) bool { // Noncompliant {{Use slices.Equal() instead of custom slice comparison}} if len(*s1) != len(s2) { return false } for i := range *s1 { if (*s1)[i] != s2[i] { return false } } return true }

And maybe you can think up more cases.

mstachniuk · 2025-09-03T12:52:07Z

.../slang-checks/src/test/resources/org/sonarsource/slang/checks/SliceEqualSimplification.slang

+}
+
+// Negative cases - should not be flagged
+fun notEqualSlicesWrongLenCheck(a, b) {


Please change this code to Go code.

mstachniuk · 2025-09-03T12:59:59Z

Hi @paulnegz !
Sorry for late review, I was off for few weeks and busy with other activities.
I wrote a lot of comments, please address them. Please ask if something is unclear.
Please be sure that you can run locally the SliceEqualSimplificationCheckTest (it is currently not the case).
After addressing the comments I will do a second round of the review, maybe someone else from my team will also look on it.
Please do not treat my comments as push back. We care about the quality of the code we deliver.
I'm happy to continue to work with you on your PR.

paulnegz mentioned this pull request Jul 30, 2025

SLANG Add rule to detect slice equality functions that can use slices… SonarSource/slang#339

Closed

mstachniuk changed the title ~~feat(checks): Add SliceEqualSimplificationCheck~~ Use "slices.Equal()" instead of custom implementation of this function Sep 3, 2025

mstachniuk added the External Contribution label Sep 3, 2025

mstachniuk requested changes Sep 3, 2025

View reviewed changes

	public class SliceEqualSimplificationCheck implements SlangCheck {
	public class SliceEqualSimplificationCheck implements GoCheck {

	Verifier.verify("SliceEqualSimplification.slang", new SliceEqualSimplificationCheck());
	GoVerifier.verify("SliceEqualSimplification/SliceEqualSimplification.go", new SliceEqualSimplificationCheck());

		@@ -0,0 +1,100 @@
		// Positive cases
		fun equalSlices(a, b) { // Noncompliant {{Use slices.Equal() instead of custom slice comparison}}

Conversation

paulnegz commented Jul 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mstachniuk commented Sep 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants