Properly set hasAction for erasing modifiers in class member completions

[gabritto]

Fixes #57301.
Also fixes #57333.
@iisaduan pointed out that class member completions were always duplicating modifiers (any modifiers, not just async).

To update on my last comment on the original PR for dealing with modifiers in class member completions, the current behavior (with this fix) is:

• We only offer the completion if the already present modifiers match the modifiers the completion would insert
• The modifiers already present may be out of order, so we always add all the modifiers as part of the member completion entry, and send a code action to erase the present modifiers.

Example:

class Base {
    protected prop = 1;
}
class E extends Base {
    override protected |
}

becomes, after accepting completion for prop:

class Base {
    protected prop = 1;
}
class E extends Base {
    protected override prop: number;|
}

[gabritto]

@Andarist the PR I mentioned that might affect #57480.

[Andarist]

I kind of already had this change as part of my PR (see the commit here). I found the code action approach to have some DX downsides (mentioned that past half point of my comment here) and thus I didn't quite follow on it to adjust the test cases, add a new diagnostic etc.

[gabritto]

As I said in #57480 (comment):

The other course of action I can think of to fix this is to omit existing modifiers from the completion, but now if the modifier order is wrong or you need to add a modifier before the existing one, you can't.
It really does look like what we need is to use a replacementSpan to replace the existing modifiers. Maybe I should check with vscode again if that's at all possible.

Meanwhile, this PR fixes what I tried to implement in #52525.

[iisaduan]

I'm still looking through the code, but the test cases look pretty good so far! I think we should add a case for #57333, since this PR also fixes it.

//b.ts
export type C = { x: number }

export interface ExtShape {
    $arg(id: C ): Promise<number[]>;
    $returnPromise(id: number): Promise<C>;
    $return(id: number): C;
}

//test.ts
import { ExtShape } from './b';

abstract class ExtBase implements ExtShape {
    public $/**/
}

[Andarist]

you could avoid having to pass previousToken here like this: Andarist@4721fbd

note that this commit reverts changes to getPresentModifiers and introduces changes (slightly adjusted) from #57480 . I think that your version might have some further readability improvements but I just skipped merging your changes into this. If you like the core of the proposed change - I can incorporate your changes into this and open a PR targeting your branch

[gabritto]

Does that work because the case where previousToken is not the same as contextToken is the same as when contextToken.parent.name !== contextToken?

[Andarist]

I believe so, yes.

[iisaduan]

Thank you and @Andarist for investigating and fixing this!