Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strings cannot be nil inspection #2871

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions resources/META-INF/gogland.xml
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,9 @@
<localInspection language="go" displayName="Multiple packages in directory declaration" groupPath="Go"
groupName="General" enabledByDefault="true" level="ERROR"
implementationClass="com.goide.inspections.GoMultiplePackagesInspection"/>
<localInspection language="go" displayName="String cannot be nil" groupPath="Go"
groupName="General" enabledByDefault="true" level="ERROR"
implementationClass="com.goide.inspections.GoStringCannotBeNilInspection"/>
<localInspection language="go" displayName="Usage of cgo in tests is not supported" groupPath="Go"
groupName="General" enabledByDefault="true" level="ERROR"
implementationClass="com.goide.inspections.GoCgoInTestInspection"/>
Expand Down
21 changes: 21 additions & 0 deletions resources/inspectionDescriptions/GoStringCannotBeNil.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<!--
~ Copyright 2013-2017 Sergey Ignatov, Alexander Zolotov, Florin Patan
~
~ Licensed under the Apache License, Version 2.0 (the "License");
~ you may not use this file except in compliance with the License.
~ You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing, software
~ distributed under the License is distributed on an "AS IS" BASIS,
~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
~ See the License for the specific language governing permissions and
~ limitations under the License.
-->

<html>
<body>
Inspects for string assignation or comparision to nil.
</body>
</html>
90 changes: 90 additions & 0 deletions src/com/goide/inspections/GoStringCannotBeNilInspection.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package com.goide.inspections;

import com.goide.GoConstants;
import com.goide.GoTypes;
import com.goide.psi.*;
import com.goide.psi.impl.GoElementFactory;
import com.goide.psi.impl.GoPsiImplUtil;
import com.goide.psi.impl.GoTypeUtil;
import com.intellij.codeInspection.*;
import com.intellij.openapi.project.Project;
import com.intellij.psi.PsiElement;
import com.intellij.psi.tree.IElementType;
import org.jetbrains.annotations.NotNull;

import java.util.List;
import java.util.stream.IntStream;

public class GoStringCannotBeNilInspection extends GoInspectionBase {
public static final String QUICK_FIX_NAME = "Change to default value";
private static final String DEFAULT_STRING = "\"\"";
private static final String PROBLEM_DESCRIPTION = "String cannot be nil";

@NotNull
@Override
protected GoVisitor buildGoVisitor(@NotNull ProblemsHolder holder, @NotNull LocalInspectionToolSession session) {
return new GoVisitor() {

@Override
public void visitVarSpec(@NotNull GoVarSpec o) {
super.visitVarSpec(o);

if (o.getVarDefinitionList() == null) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always false

o.getVarDefinitionList().forEach(var -> check(var, var != null ? var.getValue() : null));
}

@Override
public void visitAssignmentStatement(@NotNull GoAssignmentStatement o) {
super.visitAssignmentStatement(o);

if (o.getLeftHandExprList() == null) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always false

List<GoExpression> rightSide = o.getExpressionList();
List<GoExpression> leftSide = o.getLeftHandExprList().getExpressionList();
if (leftSide == null || rightSide == null) return;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

always false


IntStream.range(0, Math.min(leftSide.size(), rightSide.size()))
.forEach(i -> check(leftSide.get(i), rightSide.get(i)));
}

@Override
public void visitBinaryExpr(@NotNull GoBinaryExpr o) {
super.visitBinaryExpr(o);
if (o.getOperator() == null || o.getOperator().getNode() == null) return;
IElementType type = o.getOperator().getNode().getElementType();
if (type == GoTypes.EQ || type == GoTypes.NOT_EQ) {
check(o.getLeft(), o.getRight());
check(o.getRight(), o.getLeft());
}
}

protected void check(GoTypeOwner var, GoExpression value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why protected?


if (var == null || value == null) return;
GoType varType = var.getGoType(null);
if (!GoTypeUtil.isString(varType)) return;

if (value instanceof GoReferenceExpression && value.textMatches(GoConstants.NIL)
&& GoPsiImplUtil.builtin(((GoReferenceExpression)value).resolve())) {

holder.registerProblem(value, PROBLEM_DESCRIPTION, ProblemHighlightType.GENERIC_ERROR_OR_WARNING,
new GoChangeStringToDefaultValueQuickFix());
}
}
};
}

private static class GoChangeStringToDefaultValueQuickFix extends LocalQuickFixBase {
public GoChangeStringToDefaultValueQuickFix() {
super(QUICK_FIX_NAME);
}

@Override
public void applyFix(@NotNull Project project, @NotNull ProblemDescriptor descriptor) {
PsiElement element = descriptor.getPsiElement();
if (element == null || !element.isValid()) return;
if (element instanceof GoExpression) {
element.replace(GoElementFactory.createExpression(project, DEFAULT_STRING));
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

func main() {
var var1, var2, var3 string;
var1, var2, var3 = "nil", "", "text"
}
6 changes: 6 additions & 0 deletions testData/inspections/string-assigned-to-nil/assignment.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

func main() {
var var1, var2, var3 string;
var1, var2, var3 = "nil", <error descr="String cannot be nil"><caret>nil</error>, "text"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

func main() {
var var1 string;
"" == var1
}
6 changes: 6 additions & 0 deletions testData/inspections/string-assigned-to-nil/comparison.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

func main() {
var var1 string;
<error descr="String cannot be nil"><caret>nil</error> == var1
}
6 changes: 6 additions & 0 deletions testData/inspections/string-assigned-to-nil/nilVariable.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
package main

func main() {
nil := "text"
var1 := nil
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package main

func main() {
var var1, var2 string = "nil", "", 5 + 13
}
5 changes: 5 additions & 0 deletions testData/inspections/string-assigned-to-nil/tooManyValues.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package main

func main() {
var var1, var2 string = "nil", <error descr="String cannot be nil"><caret>nil</error>, 5 + 13
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package main

func main() {
var var1, var2, var3, var4 string = "nil", "", 5 + 13
}
5 changes: 5 additions & 0 deletions testData/inspections/string-assigned-to-nil/varDeclaration.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package main

func main() {
var var1, var2, var3, var4 string = "nil", <error descr="String cannot be nil"><caret>nil</error>, 5 + 13
}
55 changes: 55 additions & 0 deletions tests/com/goide/inspections/GoStringCannotBeNilInspectionTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2013-2017 Sergey Ignatov, Alexander Zolotov, Florin Patan
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.goide.inspections;

import com.goide.SdkAware;
import com.goide.quickfix.GoQuickFixTestBase;

@SdkAware
public class GoStringCannotBeNilInspectionTest extends GoQuickFixTestBase {

@Override
protected void setUp() throws Exception {
super.setUp();
myFixture.enableInspections(GoStringCannotBeNilInspection.class);
}

public void testVarDeclaration() {
doTest(GoStringCannotBeNilInspection.QUICK_FIX_NAME, true);
}

public void testAssignment() {
doTest(GoStringCannotBeNilInspection.QUICK_FIX_NAME, true);
}

public void testTooManyValues() {
doTest(GoStringCannotBeNilInspection.QUICK_FIX_NAME, true);
}

public void testComparison() {
doTest(GoStringCannotBeNilInspection.QUICK_FIX_NAME, true);
}

public void testNilVariable() {
myFixture.testHighlighting(getTestName(true) + ".go");
}

@Override
protected String getBasePath() {
return "inspections/string-assigned-to-nil";
}
}