ballerina-platform / ballerina-lang

The Ballerina Programming Language
https://ballerina.io/
Apache License 2.0
3.66k stars 751 forks source link

[Bug]: `Change variable type` CA is not provided for ternary expressions #42081

Open nipunayf opened 9 months ago

nipunayf commented 9 months ago

Description

Currently, there is no change variable type CA provided for ternary expressions. Ideally, it should suggest changing the variable type to the determined type of the ternary expression.

https://github.com/ballerina-platform/ballerina-lang/blob/2237ffa3f9578b3a8812c80387cca5b9c4ccc891/language-server/modules/langserver-core/src/main/java/org/ballerinalang/langserver/codeaction/providers/changetype/ChangeVariableTypeCodeAction.java#L88-L96

The current implementation of the CA considers the properties of the diagnostic to determine the expected type to which the variable should be changed. Although this works for cases where the diagnostic highlights the entire expression, this approach cannot determine the union type when there are multiple diagnostics. Hence, the current implementation struggles to provide the expected type for ternary expressions.

As a solution, we can use the semantic model to determine the expected type (as we are doing in the fix return type CA). However, this affects performance, as the majority of use cases benefit from the existing implementation. Hence, we can discuss on a heuristic to use the semantic model only for specific cases such as ternary and elvis expressions.

Steps to Reproduce

Prompt for CAs at the following cursor position.

boolean flag = false;
() output = flag ? true : <cursor>0;

Affected Version(s)

Ballerina 2201.8.4 (Swan Lake Update 8)

OS, DB, other environment details and versions

OS: macOS 14.2.1 23C71 JDK: openjdk 17.0.8 2023-07-18

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

mindula commented 7 months ago

Duplicate of #38258