Skip to content

Commit

Permalink
Merge pull request #42196 from KavinduZoysa/fix-issue-42127
Browse files Browse the repository at this point in the history
Return user-defined errors in `create variable and check error` with check CA
  • Loading branch information
KavinduZoysa authored Mar 19, 2024
2 parents 991f718 + 129ac2b commit 9215217
Show file tree
Hide file tree
Showing 134 changed files with 6,295 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,15 @@
*/
package org.ballerinalang.langserver.codeaction.providers.createvar;

import io.ballerina.compiler.api.symbols.ModuleSymbol;
import io.ballerina.compiler.api.symbols.Qualifiable;
import io.ballerina.compiler.api.symbols.Qualifier;
import io.ballerina.compiler.api.symbols.Symbol;
import io.ballerina.compiler.api.symbols.TypeDescKind;
import io.ballerina.compiler.api.symbols.TypeReferenceTypeSymbol;
import io.ballerina.compiler.api.symbols.TypeSymbol;
import io.ballerina.compiler.api.symbols.UnionTypeSymbol;
import io.ballerina.projects.Module;
import io.ballerina.tools.diagnostics.Diagnostic;
import org.ballerinalang.annotation.JavaSPIService;
import org.ballerinalang.langserver.codeaction.CodeActionNodeValidator;
Expand All @@ -34,6 +40,8 @@
import org.eclipse.lsp4j.Range;
import org.eclipse.lsp4j.TextEdit;

import java.net.URI;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
Expand Down Expand Up @@ -83,9 +91,13 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic,
return Collections.emptyList();
}
UnionTypeSymbol unionTypeDesc = (UnionTypeSymbol) typeSymbol.get();
List<TypeSymbol> errorMemberTypes = unionTypeDesc.memberTypeDescriptors().stream()
.filter(member -> CommonUtil.getRawType(member).typeKind() == TypeDescKind.ERROR)
.collect(Collectors.toList());
List<TypeSymbol> errorMemberTypes = CommonUtil.extractErrorTypesFromUnion(unionTypeDesc);
Path path = Path.of(URI.create(uri.replace("expr:///", "file:///")));
Optional<Module> module = context.workspace().module(path);
if (module.isPresent() &&
containsModuleLevelPrivateTypes(module.get().moduleName().toString(), errorMemberTypes)) {
return Collections.emptyList();
}
long nonErrorNonNilMemberCount = unionTypeDesc.memberTypeDescriptors().stream()
.filter(member -> CommonUtil.getRawType(member).typeKind() != TypeDescKind.ERROR
&& member.typeKind() != TypeDescKind.NIL)
Expand All @@ -109,6 +121,23 @@ public List<CodeAction> getCodeActions(Diagnostic diagnostic,
modifiedTextEdits.imports.size());
return Collections.singletonList(codeAction);
}

private static boolean containsModuleLevelPrivateTypes(String currentModule, List<TypeSymbol> errorMemberTypes) {
for (TypeSymbol errorMemType : errorMemberTypes) {
if (errorMemType.typeKind() != TypeDescKind.TYPE_REFERENCE) {
continue;
}
Optional<ModuleSymbol> module = errorMemType.getModule();
if (module.isEmpty() || currentModule.equals(module.get().id().moduleName())) {
continue;
}
Symbol typeDef = ((TypeReferenceTypeSymbol) errorMemType).definition();
if (typeDef instanceof Qualifiable qualifiable && !qualifiable.qualifiers().contains(Qualifier.PUBLIC)) {
return true;
}
}
return false;
}

@Override
public String getName() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.function.Predicate;
Expand Down Expand Up @@ -330,6 +331,63 @@ public static TypeSymbol getRawType(TypeSymbol typeDescriptor) {
return typeDescriptor;
}

/**
* Check whether the given type is an error type or union of error types.
*
* @param type type descriptor to evaluate
* @return {@link Boolean} is error type or union or error types
*/
public static boolean isErrorOrUnionOfErrors(TypeSymbol type) {
TypeDescKind kind = type.typeKind();
if (kind == TypeDescKind.ERROR) {
return true;
}
if (kind == TypeDescKind.TYPE_REFERENCE) {
TypeSymbol rawType = getRawType(type);
if (rawType.typeKind() == TypeDescKind.UNION) {
return ((UnionTypeSymbol) rawType).memberTypeDescriptors().stream()
.allMatch(CommonUtil::isErrorOrUnionOfErrors);
}
return isErrorOrUnionOfErrors(rawType);
}
return false;
}

private static void getErrorTypes(TypeSymbol type, TypeSymbol typeRef, List<TypeSymbol> errorTypes) {
TypeDescKind kind = type.typeKind();
if (kind == TypeDescKind.ERROR) {
errorTypes.add(Objects.requireNonNullElse(typeRef, type));
} else if (kind == TypeDescKind.TYPE_REFERENCE) {
TypeSymbol rawType = getRawType(type);
if (rawType.typeKind() == TypeDescKind.UNION) {
UnionTypeSymbol unionRawType = (UnionTypeSymbol) rawType;
if (unionRawType.memberTypeDescriptors().stream().allMatch(CommonUtil::isErrorOrUnionOfErrors)) {
errorTypes.add(type);
} else {
for (TypeSymbol memberType : unionRawType.userSpecifiedMemberTypes()) {
getErrorTypes(memberType, memberType, errorTypes);
}
}
} else {
getErrorTypes(rawType, type, errorTypes);
}
}
}

/**
* Extract member error types from the union type.
*
* @param unionType union type descriptor to evaluate
* @return {@link List<TypeSymbol>} member error types
*/
public static List<TypeSymbol> extractErrorTypesFromUnion(UnionTypeSymbol unionType) {
List<TypeSymbol> exactErrorTypes = new ArrayList<>();
for (TypeSymbol memType : unionType.userSpecifiedMemberTypes()) {
getErrorTypes(memType, null, exactErrorTypes);
}
return exactErrorTypes;
}

/**
* Check if the provided union type is a union of members of provided type desc kind.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,14 @@ public Object[][] dataProvider() {
{"createVariableWithCheck10.json"},
{"createVariableWithCheck11.json"},
{"createVariableWithCheck12.json"},
{"createVariableWithCheck13.json"}
{"createVariableWithCheck13.json"},
{"createVariableWithCheck14.json"},
{"createVariableWithCheck15.json"},
{"createVariableWithCheck16.json"},
{"createVariableWithCheck17.json"},
{"createVariableWithCheck18.json"},
{"createVariableWithCheck19.json"},
{"createVariableWithCheck20.json"},
};
}

Expand All @@ -171,7 +178,8 @@ public Object[][] negativeDataProvider() {
{"createVariableNegative2.json"},
{"createVariableNegative3.json"},
{"createVariableNegative4.json"},
{"createVariableInQueryActionNegative.json"}
{"createVariableInQueryActionNegative.json"},
{"createVariableWithCheckNegative1.json"},
};
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"position": {
"line": 5,
"character": 16
},
"source": "createVariableWithCheck3.bal",
"description": "Parent function has no return type",
"expected": [
{
"title": "Create variable and check error",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 5,
"character": 4
},
"end": {
"line": 5,
"character": 4
}
},
"newText": "int valueOrError1 = "
},
{
"range": {
"start": {
"line": 5,
"character": 4
},
"end": {
"line": 5,
"character": 4
}
},
"newText": "check "
},
{
"range": {
"start": {
"line": 4,
"character": 15
},
"end": {
"line": 4,
"character": 15
}
},
"newText": " returns Error1?"
}
],
"command": {
"title": "Rename variable",
"command": "ballerina.action.positional.rename",
"arguments": [
"createVariableWithCheck3.bal",
{
"line": 5,
"character": 8
}
]
},
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"position": {
"line": 15,
"character": 12
},
"source": "createVariableWithCheck3.bal",
"description": "Parent function has no return type",
"expected": [
{
"title": "Create variable and check error",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 15,
"character": 4
},
"end": {
"line": 15,
"character": 4
}
},
"newText": "int valueOrError2 = "
},
{
"range": {
"start": {
"line": 15,
"character": 4
},
"end": {
"line": 15,
"character": 4
}
},
"newText": "check "
},
{
"range": {
"start": {
"line": 14,
"character": 15
},
"end": {
"line": 14,
"character": 15
}
},
"newText": " returns Error1|E3?"
}
],
"command": {
"title": "Rename variable",
"command": "ballerina.action.positional.rename",
"arguments": [
"createVariableWithCheck3.bal",
{
"line": 15,
"character": 8
}
]
},
"resolvable": false
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"position": {
"line": 21,
"character": 13
},
"source": "createVariableWithCheck3.bal",
"description": "Parent function has no return type",
"expected": [
{
"title": "Create variable and check error",
"kind": "quickfix",
"edits": [
{
"range": {
"start": {
"line": 21,
"character": 4
},
"end": {
"line": 21,
"character": 4
}
},
"newText": "int valueOrError3 = "
},
{
"range": {
"start": {
"line": 21,
"character": 4
},
"end": {
"line": 21,
"character": 4
}
},
"newText": "check "
},
{
"range": {
"start": {
"line": 20,
"character": 15
},
"end": {
"line": 20,
"character": 15
}
},
"newText": " returns Error1|E3?"
}
],
"command": {
"title": "Rename variable",
"command": "ballerina.action.positional.rename",
"arguments": [
"createVariableWithCheck3.bal",
{
"line": 21,
"character": 8
}
]
},
"resolvable": false
}
]
}
Loading

0 comments on commit 9215217

Please sign in to comment.