ballerina-lang icon indicating copy to clipboard operation
ballerina-lang copied to clipboard

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

Open nipunayf opened this issue 1 year ago • 2 comments

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)

https://github.com/ballerina-platform/ballerina-lang/issues/28681

Suggested label(s) (optional)

No response

Suggested assignee(s) (optional)

No response

nipunayf avatar Mar 15 '24 11:03 nipunayf

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.

MaryamZi avatar Apr 26 '24 05:04 MaryamZi

The Fix1 is blocked by https://github.com/ballerina-platform/ballerina-lang/issues/42725

nipunayf avatar May 09 '24 11:05 nipunayf