Powershell: Class : Methods and properties can't have the same name

[AlexBillon07]

Steps to reproduce

class TestClass {
    [string] hidden $prop

    [string]Prop() {
        return $this.prop
    }
}

$obj = [TestClass]::new()

Expected behavior

Get an error saying you can't have a property and a method with the same name

Actual behavior

PS C:\EDICore> $obj | Get-Member


   TypeName: TestClass

Name        MemberType Definition
----        ---------- ----------
Equals      Method     bool Equals(System.Object obj)
GetHashCode Method     int GetHashCode()
GetType     Method     type GetType()
Prop        Method     string Prop()
ToString    Method     string ToString()


PS C:\EDICore> $obj.Prop()
Method invocation failed because [TestClass] does not contain a method named 'Prop'.
At line:1 char:1
+ $obj.Prop()
+ ~~~~~~~~~~~
+ CategoryInfo          : InvalidOperation: (:) [], RuntimeException
+ FullyQualifiedErrorId : MethodNotFound

Environment data

> $PSVersionTable
Name                           Value
----                           -----
PSVersion                      6.1.0
PSEdition                      Core
GitCommitId                    6.1.0
OS                             Microsoft Windows 6.1.7601 S
Platform                       Win32NT
PSCompatibleVersions           {1.0, 2.0, 3.0, 4.0...}
PSRemotingProtocolVersion      2.3
SerializationVersion           1.1.0.1
WSManStackVersion              3.0

[iSazonov]

/cc @rjmholt For your collection.

[rjmholt]

C# disallows this; try compiling the following:

    class Program
    {
        static string Main { get; } = "Hi";

        static void Main(string[] args)
        {
            Console.WriteLine($"{Main} World!");
        }
    }

From The C# Language Specification - ECMA 334:

17.2 Class members

The name of a constant, field, property, event, or type must differ from the names of all other members declared in the same class.

That doesn't necessarily mean PowerShell must also, although I think it's indicative.

The Common Language Infrastructure Specification - ECMA 335 has the following to say:

I.10.2 Overloading

...
Following the rules of the CTS, it is possible for duplicate names to be defined in the same scope
as long as they differ in either kind (field, method, etc.) or signature.
...

CLS Rule 5

All names introduced in a CLS-compliant scope shall be distinct independent of
kind, except where the names are identical and resolved via overloading. That is, while the CTS
allows a single type to use the same name for a method and a field, the CLS does not.

I think it's fair to say that CLS compliance is no longer a consideration. But, given that C# doesn't allow this, I strongly suspect that the current behaviour is by design. Maybe @BrucePay or @lzybkr can shed some light there?

[rjmholt]

The fact that the method Prop() is actually registered looks like a bug though -- we should probably fix that and turn this into a specific parse-time error (e.g. Multiple class members with the same name are not allowed ☝️).

[rjmholt]

/cc @PowerShell/powershell-committee for discussion

[mklement0]

@rjmholt: +1 on the parse-time error.

Allowing members of different types to have the same name is asking for trouble with no obvious benefit (and wer're not breaking anything, given that the method cannot be called).

Note that C# does allow names that differ in _casing_, which we should disallow as well, given PowerShell's case-insensitive nature.

[IISResetMe]

@mklement0 @rjmholt looks like this is fairly straightforward to catch at parse-time: https://github.com/IISResetMe/PowerShell/commit/bfd4be766246bdf33a0ffbb027ba66723fe06b49

[mklement0]

Cool, @IISResetMe, thanks for taking this on.

Sounds like @lzybkr is already advising you on the implementation (which I'd be ill-equipped to do anyway).

One suggestion re the user-facing part: How about mimicking the wording of the C# error message, so as to include the type name too?

The type 'myclass' already contains a definition for 'A'

Though spelling out method or property can't hurt:

The type 'myclass' already contains a definition for method or property 'A'

[IISResetMe]

@mklement0 moving the mutual check back to TypeDefiner (DefineProperty() and DefineMethod() respectively) makes it way faster and avoids attempts at generating invalid/unusable types should someone pass in an AST generated by a different parser (or by hand)

https://github.com/IISResetMe/PowerShell/commit/5404f94f74af52afbf9630dbfb4a3132ff59c610#diff-640a7359770582f83b945ef2a276d2de

[SteveL-MSFT]

The PR was already reviewed by @PowerShell/powershell-committee so updating label https://github.com/PowerShell/PowerShell/pull/8372#issuecomment-454982008