Including righmost column when checking is-within-the-block

[dulajdilshan]

If I have kept my cursor in the below position.

string<cursor>

the check to check whether the cursor is within the "string" block returns false (the current implementation). Shouldn't this be true?

Ref:

ballerina-lang/compiler/ballerina-lang/src/main/java/io/ballerina/compiler/api/impl/PositionUtil.java

Lines 44 to 48 in b678383

	return (startLine < cursorLine && endLine > cursorLine)
	|| (startLine < cursorLine && endLine == cursorLine && endColumn > cursorColumn)
	|| (startLine == cursorLine && endLine > cursorLine)
	|| (startLine == endLine && startLine == cursorLine
	&& startColumn <= cursorColumn && endColumn > cursorColumn);

The origin of this issue came with the following ballerina code snippet.

type Student record {
    string firstName;
    string lastName;
    int intakeYear;
    float gpa;
};

public function main() {
    Student s1 = {
        firstName: "Michelle",
        lastName: "Sadler",
        intakeYear: 1990,
        gpa: 3.5
    };

    Student[] studentList = [s1];
    var reportList = from Student student in studentList
                    where s<cursor>
}

(see the cursor position near the where)
In this example, the issue of not getting student into the suggestions is the cursor position is considered as Not Within the Block of where-clause

Related issue: #28702

[IMS94]

@nadeeshaan @dulajdilshan I had the same requirement in #28754 because the SymbolFinder was failing to identify the highlighted symbol. When highlighted, the cursor was at the end of the symbol similar to this. SymbolFinder internally relies on PositionUtil to check within range/block. Therefore, if we can include the rightmost index, the workaround in #28754 can be reverted as well.

However, back then when I proposed the same, we did not change the PositionUtil. Cannot remember why.

[dulajdilshan]

There are some test failure if we change the behaviour of PositionUtil

[IMS94]

That's expected. We will have to fix them if we continue with this approach.

[pubudu91]

This was done intentionally. This check is left-inclusive and right-exclusive. This is done so that there isn't any overlapping boundaries which would mean a cursor position could match to multiple constructs. When you say string<cursor> the cursor is not at the string block. For example, consider string<cursor> s;. The cursor points to the whitespace between string and s;. If we make this right inclusive as well, the aforementioned cursor position is now ambiguous. Does it point to g in string or does it point to the whitespace before s;?

We had this chat in the early days of the semantic API and decided to go with left-inclusive, right-exclusive approach. Originally it was picked because it was the intuitive approach. For example, say there's a var decl: string <cursor>foo = "FOO";. Here, the cursor is at a position where one would naturally think is the starting position of the identifier foo. Having a left-exclusive, right-inclusive approach would mean an API like symbol() would return empty for this particular case, but would return the symbol for the following case: string f<cursor>oo = "FOO";

I'll have a look at this query expr usage and see if this can be resolved in another way.

[nadeeshaan]

Can’t we use different position rule approach for this visible symbol use-case? Your argument is valid for the symbol API and I’m +1 for this approach for the symbol API. But for the visible symbol approach we should consider the left inclusive and right exclusive approach as we did for the auto-completions. This is because for the cases such as let expression we define at left and refer at right. This concept can incorporate for the visible symbol resolving