Better ranges for implicit inherit error reporting.

[edgarfgp]

Description

Better ranges for implicit Inherit error reporting.

Before

After

type ClassA() = class end

type ClassB() = class end

type ClassC() = class end

type Class() =
    inherit ClassA()
    inherit ClassB()
            ^^^^^^
    inherit ClassC()
            ^^^^^^

Checklist

• Test cases added
• Release notes entry updated

[github-actions]

❗ Release notes required

---

✅ Found changes and release notes in following paths:

Change path	Release notes path	Description
src/Compiler	docs/release-notes/.FSharp.Compiler.Service/9.0.200.md

[psfinaki]

Thanks Edgar!

[edgarfgp]

Please wait for https://github.com/dotnet/fsharp/pull/17905/files to be merged first.

[auduchinok]

@edgarfgp Thanks for your contribution! I'm sorry to say this, but I disagree with this change. I'll try to explain it on this example:

type T() =
    inherit A()
    inherit B()

I expect that there's an error about multiple inherit members. That means that the actual source of the errors are the inherit members themselves, not the type names inside of them. Moving the error to the inherited type name may say to many people that there's no error with the inherit member itself, and there's something wrong with the inherited type usage.

[auduchinok]

Could you please also add tests for multiple base interfaces, if there're none?

type I =
    inherit IA
    inherit IB

[edgarfgp]

@edgarfgp Thanks for your contribution! I'm sorry to say this, but I disagree with this change. I'll try to explain it on this example:

type T() =
    inherit A()
    inherit B()

I expect that there's an error about multiple inherit members. That means that the actual source of the errors are the inherit members themselves, not the type names inside of them. Moving the error to the inherited type name may say to many people that there's no error with the inherit member itself, and there's something wrong with the inherited type usage.

@auduchinok No need to apologise for disagreeing. This is a collaborative effort and Im glad that you are reviewing my PRs :)

So is the following what you would expect ?

[<Fact>]
    let ``Types cannot inherit from multiple concrete types.`` () =
        Fsx """
type ClassA() = class end

type ClassB() = class end

type ClassC() = class end

type Class() =
    inherit ClassA()
    inherit ClassB()
    inherit ClassC()
        """
        |> typecheck
        |> shouldFail
        |> withDiagnostics [
            (Error 959, Line 10, Col 5, Line 10, Col 12, "Type definitions may only have one 'inherit' specification and it must be the first declaration")
            (Error 959, Line 11, Col 5, Line 11, Col 12, "Type definitions may only have one 'inherit' specification and it must be the first declaration")
            (Error 932, Line 10, Col 13, Line 10, Col 19, "Types cannot inherit from multiple concrete types")
            (Error 932, Line 11, Col 13, Line 11, Col 19, "Types cannot inherit from multiple concrete types")
        ]

[auduchinok]

@edgarfgp More or less, yes. I'd probably prefer the 932 errors to be on the Class type name, not on the inherit members.

[edgarfgp]

@edgarfgp More or less, yes. I'd probably prefer the 932 errors to be on the Class type name, not on the inherit members.

Humm. But using the type name and not the inherit range makes it look like multiple inherits are allowed(Unless you hover the type name). I would prefer to be use the accurate range.

[auduchinok]

Humm. But using the type name and not the inherit range makes it look like multiple inherits are allowed

Hm, it should not, since 959 errors are still going to be on the inherit members.

[edgarfgp]

@edgarfgp Thanks for your contribution! I'm sorry to say this, but I disagree with this change. I'll try to explain it on this example:

type T() =
    inherit A()
    inherit B()

I expect that there's an error about multiple inherit members. That means that the actual source of the errors are the inherit members themselves, not the type names inside of them. Moving the error to the inherited type name may say to many people that there's no error with the inherit member itself, and there's something wrong with the inherited type usage.

@auduchinok Updated following you suggestion. @abonie also suggested the same previously.

[auduchinok]

@edgarfgp Thanks!