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

[Improvement]: Remove adding type descriptor in the extract to constant code action #42041

Open
LakshanWeerasinghe opened this issue Jan 26, 2024 · 2 comments · May be fixed by #42643
Open

[Improvement]: Remove adding type descriptor in the extract to constant code action #42041

LakshanWeerasinghe opened this issue Jan 26, 2024 · 2 comments · May be fixed by #42643
Assignees
Labels
Area/CodeAction Language Server Code Actions Team/LanguageServer Language Server Implementation related issues. #Compiler Type/Improvement

Comments

@LakshanWeerasinghe
Copy link
Contributor

Description

$subject

According to the spec when declaring constant it is optional to add the type descriptor. The current compiler has the capability to resolve the constant type without the type descriptor. Also when don't have the type-descriptor the compiler don't have to type check the expression against the constant expected type.
Hence we can stop adding the type descriptor in the extract to constant code action.

https://ballerina.io/spec/lang/master/#section_8.8

module-const-decl :=
   metadata
   [public] const [type-descriptor] identifier = const-expr ;

Describe your problem(s)

No response

Describe your solution(s)

No response

Related area

-> Compilation

Related issue(s) (optional)

No response

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@LakshanWeerasinghe LakshanWeerasinghe added Type/Improvement Team/LanguageServer Language Server Implementation related issues. #Compiler labels Jan 26, 2024
@ballerina-bot ballerina-bot added needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Jan 26, 2024
@LakshanWeerasinghe LakshanWeerasinghe added Area/CodeAction Language Server Code Actions and removed needTriage The issue has to be inspected and labeled manually userCategory/Compilation labels Jan 26, 2024
@nipunayf
Copy link
Contributor

nipunayf commented Apr 24, 2024

Bumping this issue as the style guide has been updated in compliant to the above changes, and the LS should provide solutions that reflect the best practices. ballerina-platform/ballerina-dev-website#8884.

@nipunayf
Copy link
Contributor

Simply removing the type symbol is not straightforward, as the code action may generate erroneous code in some scenarios. For instance, consider the following scenario, which generates an incompatible error.

function foo(decimal val) {}

// Extracts the following value
foo(12)

// Generates the following code
const CONST = 12 // which is inferred as int

The ideal solution would be to manually consider such cases and add the d prefix to such values. However, this imposes a performance bottleneck on the code action, and the scalability is also fragile since there can be more such cases once we expand the support for constants.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area/CodeAction Language Server Code Actions Team/LanguageServer Language Server Implementation related issues. #Compiler Type/Improvement
Projects
Status: PR Sent
3 participants