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]: Provide a quick-fix code action for invalid transfer out of a lock statement #42333

Open
nipunayf opened this issue Mar 15, 2024 · 2 comments · May be fixed by #42734
Open

[Improvement]: Provide a quick-fix code action for invalid transfer out of a lock statement #42333

nipunayf opened this issue Mar 15, 2024 · 2 comments · May be fixed by #42734
Assignees
Labels
Area/CodeAction Language Server Code Actions Team/LanguageServer Language Server Implementation related issues. #Compiler Type/Improvement
Milestone

Comments

@nipunayf
Copy link
Contributor

nipunayf commented Mar 15, 2024

Description

For the diagnostic invalid attempt to transfer out a value from a 'lock' statement with restricted variable usage: expected an isolated expression, there are common fixes in which you can either return a clone of the selected value or make the respective data storage immutable. All these CAs should be provided to the user so that they can make the decision based on their use case.

Describe your problem(s)

No response

Describe your solution(s)

Consider the following source code with the respective diagnostic.

isolated map<map<string>> mp = {};

isolated function isolatedFn() returns map<string> {
    lock {
        return mp.get("a");
    }
}

FIX1: Make the data structure immutable, assuming there are no updates to the storage.

isolated map<map<string>> & readonly mp = {}; // changed

isolated function isolatedFn() returns map<string> {
    lock {
        return mp.get("a");
    }
}

FIX2: Clone the value expression, and let the caller manipulate the cloned value.

isolated map<map<string>> mp = {};

isolated function isolatedFn() returns map<string> {
    lock {
        return mp.get("a").clone(); // changed
    }
}

FIX3: Clone the value expression, and disallow the caller to manipulate the cloned value.

isolated map<map<string>> mp = {};

isolated function isolatedFn() returns map<string> {
    lock {
        return mp.get("a").cloneReadonly(); // changed
    }
}

Related area

-> Compilation

Related issue(s) (optional)

#28681

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

@nipunayf nipunayf added Type/Improvement Team/LanguageServer Language Server Implementation related issues. #Compiler Area/CodeAction Language Server Code Actions labels Mar 15, 2024
@nipunayf nipunayf self-assigned this Mar 15, 2024
@MaryamZi
Copy link
Member

MaryamZi commented Apr 26, 2024

Re: FIX1, the entire container doesn't have to be immutable, just the member. That may be a better suggestion in that case.

// the map itself is still mutable, only the members should be immutable
isolated map<map<string> & readonly> mp = {};

isolated function isolatedFn() returns map<string> {
    lock {
        return mp.get("a");
    }
}

Also, priority-wise, I think the clone options are generally more applicable.

@nipunayf
Copy link
Contributor Author

nipunayf commented May 9, 2024

The Fix1 is blocked by #42725

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
2 participants