Skip to content

Commit

Permalink
Merge pull request #43135 from nipunayf/fix-42335
Browse files Browse the repository at this point in the history
Provide code actions to fix invalid transferring of an isolated value into a lock statement
  • Loading branch information
nipunayf authored Jul 31, 2024
2 parents a4c744e + 8ede703 commit 7dbb5d5
Show file tree
Hide file tree
Showing 71 changed files with 2,596 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic,
}

// Check if there are multiple diagnostics of the considered category
List<Diagnostic> diagnostics = context.diagnostics(context.filePath());
if (diagnostics.size() > 1 && hasMultipleDiagnostics(nonTerminalNode, diagnostic, diagnostics)) {
if (CommonUtil.hasMultipleDiagnostics(context, nonTerminalNode, diagnostic, DIAGNOSTIC_CODES,
DIAGNOSTIC_CODE_3961)) {
return Collections.emptyList();
}

Expand Down Expand Up @@ -170,14 +170,6 @@ private static List<CodeAction> getCodeAction(LineRange lineRange, String expres
CodeActionUtil.createCodeAction(commandTitle, List.of(textEdit), filePath, CodeActionKind.QuickFix));
}

private static boolean hasMultipleDiagnostics(NonTerminalNode node, Diagnostic currentDiagnostic,
List<Diagnostic> diagnostics) {
return diagnostics.stream().anyMatch(diagnostic -> !currentDiagnostic.equals(diagnostic) &&
DIAGNOSTIC_CODES.contains(diagnostic.diagnosticInfo().code()) &&
PositionUtil.isWithinLineRange(diagnostic.location().lineRange(), node.lineRange())) &&
currentDiagnostic.diagnosticInfo().code().equals(DIAGNOSTIC_CODE_3961);
}

@Override
public String getName() {
return NAME;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.ballerinalang.langserver.codeaction.CodeActionNodeValidator;
import org.ballerinalang.langserver.codeaction.CodeActionUtil;
import org.ballerinalang.langserver.common.constants.CommandConstants;
import org.ballerinalang.langserver.common.utils.CommonUtil;
import org.ballerinalang.langserver.common.utils.PositionUtil;
import org.ballerinalang.langserver.commons.CodeActionContext;
import org.ballerinalang.langserver.commons.codeaction.spi.DiagBasedPositionDetails;
Expand All @@ -43,6 +44,7 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;

/**
* Code action to add readonly to the type.
Expand All @@ -53,12 +55,13 @@
public class AddReadonlyCodeAction implements DiagnosticBasedCodeActionProvider {

private static final String NAME = "Add readonly to the type";
private static final String DIAGNOSTIC_CODE = "BCE3959";
private static final String DIAGNOSTIC_CODE_3959 = "BCE3959";
private static final Set<String> DIAGNOSTIC_CODES = Set.of(DIAGNOSTIC_CODE_3959, "BCE3960");

@Override
public boolean validate(Diagnostic diagnostic, DiagBasedPositionDetails positionDetails,
CodeActionContext context) {
return DIAGNOSTIC_CODE.equals(diagnostic.diagnosticInfo().code())
return DIAGNOSTIC_CODES.contains(diagnostic.diagnosticInfo().code())
&& CodeActionNodeValidator.validate(context.nodeAtRange());
}

Expand All @@ -71,6 +74,12 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic, DiagBasedPositionD
TypeSymbol typeSymbol = semanticModel.typeOf(node).orElseThrow();
Location location = typeSymbol.getLocation().orElseThrow();

// Check if there are multiple diagnostics of the considered category
if (CommonUtil.hasMultipleDiagnostics(context, node, diagnostic, DIAGNOSTIC_CODES,
DIAGNOSTIC_CODE_3959)) {
return Collections.emptyList();
}

// Skip if a type reference, as the symbol does not contain the location of the variable initialization.
if (typeSymbol.typeKind() == TypeDescKind.TYPE_REFERENCE) {
return Collections.emptyList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.ballerinalang.langserver.codeaction.CodeActionNodeValidator;
import org.ballerinalang.langserver.codeaction.CodeActionUtil;
import org.ballerinalang.langserver.common.constants.CommandConstants;
import org.ballerinalang.langserver.common.utils.CommonUtil;
import org.ballerinalang.langserver.common.utils.PositionUtil;
import org.ballerinalang.langserver.commons.CodeActionContext;
import org.ballerinalang.langserver.commons.codeaction.spi.DiagBasedPositionDetails;
Expand All @@ -39,6 +40,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import java.util.Set;

/**
* Code action to clone the value or clone it as a readonly value.
Expand All @@ -49,14 +51,15 @@
public class CloneValueCodeAction implements DiagnosticBasedCodeActionProvider {

private static final String NAME = "Clone the value";
private static final String DIAGNOSTIC_CODE = "BCE3959";
private static final String DIAGNOSTIC_CODE_3959 = "BCE3959";
private static final Set<String> DIAGNOSTIC_CODES = Set.of(DIAGNOSTIC_CODE_3959, "BCE3960");
private static final String CLONE_METHOD = ".clone()";
private static final String CLONE_READONLY_METHOD = ".cloneReadOnly()";

@Override
public boolean validate(Diagnostic diagnostic, DiagBasedPositionDetails positionDetails,
CodeActionContext context) {
return DIAGNOSTIC_CODE.equals(diagnostic.diagnosticInfo().code())
return DIAGNOSTIC_CODES.contains(diagnostic.diagnosticInfo().code())
&& CodeActionNodeValidator.validate(context.nodeAtRange());
}

Expand All @@ -65,6 +68,12 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic, DiagBasedPositionD
CodeActionContext context) {
NonTerminalNode matchedNode = positionDetails.matchedNode();

// Check if there are multiple diagnostics of the considered category
if (CommonUtil.hasMultipleDiagnostics(context, matchedNode, diagnostic, DIAGNOSTIC_CODES,
DIAGNOSTIC_CODE_3959)) {
return Collections.emptyList();
}

// Obtain the type symbol of the matched node.
Optional<TypeSymbol> optTypeSymbol = context.currentSemanticModel()
.flatMap(semanticModel -> semanticModel.typeOf(positionDetails.matchedNode()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,15 @@
import io.ballerina.compiler.syntax.tree.SyntaxTree;
import io.ballerina.compiler.syntax.tree.Token;
import io.ballerina.projects.Package;
import io.ballerina.tools.diagnostics.Diagnostic;
import io.ballerina.tools.text.LinePosition;
import io.ballerina.tools.text.LineRange;
import io.ballerina.tools.text.TextDocument;
import io.ballerina.tools.text.TextRange;
import org.apache.commons.lang3.SystemUtils;
import org.ballerinalang.langserver.LSPackageLoader;
import org.ballerinalang.langserver.commons.BallerinaCompletionContext;
import org.ballerinalang.langserver.commons.CodeActionContext;
import org.ballerinalang.langserver.commons.DocumentServiceContext;
import org.ballerinalang.langserver.commons.completion.LSCompletionItem;
import org.ballerinalang.langserver.completions.SymbolCompletionItem;
Expand Down Expand Up @@ -756,6 +758,28 @@ public static Optional<Token> getLastQualifier(BallerinaCompletionContext contex
return Optional.of(qualifiers.get(qualifiers.size() - 1));
}

/**
* Checks if there are multiple diagnostics for the considered code action.
*
* @param context The code action context
* @param node The non-terminal node around which the diagnostics are being checked
* @param currentDiagnostic The current diagnostic being evaluated
* @param diagnostics The diagnostics relevant to the code action
* @param prioritizedDiagnostic The prioritized diagnostic that won't being ignored.
* @return {@code true} if there are multiple diagnostics for the relevant code action.
*/
public static boolean hasMultipleDiagnostics(CodeActionContext context, NonTerminalNode node,
Diagnostic currentDiagnostic,
Set<String> diagnostics,
String prioritizedDiagnostic) {
List<Diagnostic> projectDiagnostics = context.diagnostics(context.filePath());
return projectDiagnostics.size() > 1 &&
projectDiagnostics.stream().anyMatch(diagnostic -> !currentDiagnostic.equals(diagnostic) &&
diagnostics.contains(diagnostic.diagnosticInfo().code()) &&
PositionUtil.isWithinLineRange(diagnostic.location().lineRange(), node.lineRange())) &&
currentDiagnostic.diagnosticInfo().code().equals(prioritizedDiagnostic);
}

/**
* Returns the matching node for a given node.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,30 @@ public Object[][] dataProvider() {
{"add_readonly_config22.json"},
{"add_readonly_config23.json"},
{"add_readonly_config24.json"},
{"add_readonly_config25.json"},
{"add_readonly_config26.json"},
{"add_readonly_config27.json"},
{"add_readonly_config28.json"},
{"add_readonly_config29.json"},
{"add_readonly_config30.json"},
{"add_readonly_config31.json"},
{"add_readonly_config32.json"},
{"add_readonly_config33.json"},
{"add_readonly_config34.json"},
{"add_readonly_config35.json"},
{"add_readonly_config36.json"},
{"add_readonly_config37.json"},
{"add_readonly_config38.json"},
{"add_readonly_config39.json"},
{"add_readonly_config40.json"},
{"add_readonly_config41.json"},
{"add_readonly_config42.json"},
{"add_readonly_config43.json"},
{"add_readonly_config44.json"},
{"add_readonly_config45.json"},
{"add_readonly_config46.json"},
{"add_readonly_config47.json"},
{"add_readonly_config48.json"}
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,34 @@ public Object[][] dataProvider() {
{"clone_value_config25.json"},
{"clone_value_config26.json"},
{"clone_value_config27.json"},
{"clone_value_config28.json"}
{"clone_value_config28.json"},
{"clone_value_config29.json"},
{"clone_value_config30.json"},
{"clone_value_config31.json"},
{"clone_value_config32.json"},
{"clone_value_config33.json"},
{"clone_value_config34.json"},
{"clone_value_config35.json"},
{"clone_value_config36.json"},
{"clone_value_config37.json"},
{"clone_value_config38.json"},
{"clone_value_config39.json"},
{"clone_value_config40.json"},
{"clone_value_config41.json"},
{"clone_value_config42.json"},
{"clone_value_config43.json"},
{"clone_value_config44.json"},
{"clone_value_config45.json"},
{"clone_value_config46.json"},
{"clone_value_config47.json"},
{"clone_value_config48.json"},
{"clone_value_config49.json"},
{"clone_value_config50.json"},
{"clone_value_config51.json"},
{"clone_value_config52.json"},
{"clone_value_config53.json"},
{"clone_value_config54.json"},
{"clone_value_config55.json"}
};
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"position": {
"line": 7,
"character": 15
},
"source": "add_readonly7.bal",
"description": "Add readonly to a map value",
"expected": [
{
"title": "Add readonly to 'map<string>'",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 5,
"character": 33
},
"end": {
"line": 5,
"character": 33
}
},
"newText": " & readonly"
}
],
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"position": {
"line": 13,
"character": 15
},
"source": "add_readonly7.bal",
"description": "Add readonly to a map value",
"expected": [
{
"title": "Add readonly to 'map<string|int>'",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 11,
"character": 37
},
"end": {
"line": 11,
"character": 37
}
},
"newText": " & readonly"
}
],
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"position": {
"line": 19,
"character": 15
},
"source": "add_readonly7.bal",
"description": "Add readonly to a map value",
"expected": [
{
"title": "Add readonly to 'map<map<string>>'",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 17,
"character": 38
},
"end": {
"line": 17,
"character": 38
}
},
"newText": " & readonly"
}
],
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"position": {
"line": 25,
"character": 20
},
"source": "add_readonly7.bal",
"description": "Add readonly to a map value",
"expected": [
{
"title": "Add readonly to 'map<string>'",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 23,
"character": 33
},
"end": {
"line": 23,
"character": 33
}
},
"newText": " & readonly"
}
],
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"position": {
"line": 31,
"character": 30
},
"source": "add_readonly7.bal",
"description": "Add readonly to a map value",
"expected": [
{
"title": "Add readonly to 'map<string[]>'",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 29,
"character": 35
},
"end": {
"line": 29,
"character": 35
}
},
"newText": " & readonly"
}
],
"resolvable": false
}
]
}
Loading

0 comments on commit 7dbb5d5

Please sign in to comment.