Unify the way anonymous class symbols are printed in error messages#731
Unify the way anonymous class symbols are printed in error messages#731Andarist wants to merge 6 commits intomicrosoft:mainfrom
Conversation
...a/baselines/reference/submodule/conformance/privateNameMethodClassExpression.errors.txt.diff
Show resolved
Hide resolved
| C.getInstance().#field; // Error | ||
| ~~~~~~ | ||
| !!! error TS18013: Property '#field' is not accessible outside class '(Missing)' because it has a private identifier. | ||
| !!! error TS18013: Property '#field' is not accessible outside class 'C' because it has a private identifier. |
There was a problem hiding this comment.
Strada would print (anonymous) here but this has an inferrable~ name from the declaration and I think it's better to use it. Using it is not a new thing to do.
| !!! error TS2322: Type '(Anonymous class)' is not assignable to type 'A'. | ||
| !!! error TS2322: Property '#foo' in type '(Anonymous class)' refers to a different member that cannot be accessed from within type 'A'. |
There was a problem hiding this comment.
Strada would print (Anonymous class) in the first line here and then (anonymous) in the second line. So I think this change unifying it is a good thing.
Note this is a new test case, not one from Strada.
| privateIdentifierDescription := unmatchedProperty.ValueDeclaration.Name().Text() | ||
| symbolTableKey := binder.GetSymbolNameForPrivateIdentifier(source.symbol, privateIdentifierDescription) | ||
| if r.c.getPropertyOfType(source, symbolTableKey) != nil { | ||
| sourceName := scanner.DeclarationNameToString(ast.GetNameOfDeclaration(source.symbol.ValueDeclaration)) |
There was a problem hiding this comment.
the previous behavior here was already diverging slightly from Strada for those anonymous classes (IMHO, in a good way)
|
I'm not 100% clear why |
|
This isn't particularly consistent: TS playground. I am surprised now that the first error mentions
|
|
Old PR, but it would be worth updating this PR I think, at least into a form that reduces baseline diffs. I'd rather that be the outcome. |
…-print # Conflicts: # testdata/baselines/reference/submoduleAccepted/conformance/privateNameMethodClassExpression.errors.txt.diff # testdata/submoduleAccepted.txt
|
@jakebailey done |
|
Er, well sort of, but it's not reducing diffs, it's still changing the behavior. |
|
It changes one diff and adds it to the accepted changes. I could port this faithfully (to print |
jakebailey
left a comment
jakebailey
left a comment
There was a problem hiding this comment.
On second thought, I feel like this is a reasonable improvement, however it would probably make sense to stick this in Strada too, given the improvement should apply there?
|
Sure, I can port this to Strada |
|
I created a PR porting this to Strada: microsoft/TypeScript#61943 so I have reverted new tests etc from here given they will come to Corsa automatically once you bump the submodule after that port lands |
|
I'd leave the tests here; it's fine for tests to be "duplicated" between the two temporarily. The names will not conflict. |
2 participants
This is slightly different from Strada in two ways:
GetNameOfDeclarationis preferred now(anonymous)becomes(Anonymous class)- which is just more consistent