Fix checks for disallowed property/accessor duplicate declarations

[ahejlsberg]

With this PR the binder consistently permits accessor and property declarations to merge, and disallowed patterns are detected and reported by the checker. Specifically, we disallow duplicate property declarations in the same type declaration, and we disallow combinations of accessor and property declarations for the same name in the same type declaration.

Fixes #3613.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adjusts symbol binding and checker logic so accessor/property declarations can legally merge at bind time, while the checker reports disallowed duplicate patterns more consistently (fixing #3613).

Changes:

• Updated checker duplicate-member detection to distinguish property vs accessor combinations within a single object type declaration.
• Tweaked binder/symbol-flag exclusion rules to allow merging between properties and accessors, and adjusted type lookup to prefer accessor-derived types when a symbol is marked as an accessor.
• Refreshed and added baselines to reflect the new diagnostics, symbol graphs, and inferred types; removed now-unneeded submoduleAccepted diffs.

Reviewed changes

Copilot reviewed 40 out of 40 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
testdata/submoduleAccepted.txt	Drops baselines that are no longer expected to differ under submoduleAccepted.
testdata/baselines/reference/submoduleAccepted/compiler/gettersAndSettersErrors.types.diff	Removes obsolete accepted diff after behavior change.
testdata/baselines/reference/submoduleAccepted/compiler/gettersAndSettersErrors.symbols.diff	Removes obsolete accepted diff after behavior change.
testdata/baselines/reference/submoduleAccepted/compiler/gettersAndSettersErrors.errors.txt.diff	Updates accepted error diff output alignment/counts.
testdata/baselines/reference/submoduleAccepted/compiler/duplicateClassElements.types.diff	Updates accepted diff to reflect new merged symbol/type behavior.
testdata/baselines/reference/submoduleAccepted/compiler/duplicateClassElements.symbols.diff	Updates accepted diff symbol merging output.
testdata/baselines/reference/submoduleAccepted/compiler/duplicateClassElements.errors.txt.diff	Updates accepted diff diagnostics output ordering/counts.
testdata/baselines/reference/submoduleAccepted/compiler/allowJscheckJsTypeParameterNoCrash.symbols.diff	Updates accepted diff for symbol declaration attribution changes.
testdata/baselines/reference/submodule/conformance/propertyAndAccessorWithSameName.types.diff	Adds new diff baseline for updated inferred types.
testdata/baselines/reference/submodule/conformance/propertyAndAccessorWithSameName.types	Updates baseline types to match new accessor/property merge behavior.
testdata/baselines/reference/submodule/conformance/propertyAndAccessorWithSameName.symbols.diff	Adds new diff baseline for merged symbol declarations.
testdata/baselines/reference/submodule/conformance/propertyAndAccessorWithSameName.symbols	Updates baseline symbols for merged declarations.
testdata/baselines/reference/submodule/conformance/privateNameDuplicateField.symbols.diff	Updates diff baseline for merged private-name symbol declarations.
testdata/baselines/reference/submodule/conformance/privateNameDuplicateField.symbols	Updates baseline symbols for merged private-name declarations.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).types.diff	Adds new diff baseline reflecting updated property/accessor type selection.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).types	Updates baseline types after accessor/property merge handling changes.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).symbols.diff	Adds new diff baseline showing merged symbols for object literal members.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).symbols	Updates baseline symbol output for object literals.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).errors.txt.diff	Adds new diff baseline reflecting removed redundant TS2300s and new error totals.
testdata/baselines/reference/submodule/conformance/objectLiteralErrors(target=es2015).errors.txt	Updates baseline errors for object literals to match new diagnostic set.
testdata/baselines/reference/submodule/conformance/autoAccessor11.symbols.diff	Adds new diff baseline for updated auto-accessor symbol merging.
testdata/baselines/reference/submodule/conformance/autoAccessor11.symbols	Updates baseline symbols for auto-accessor merging.
testdata/baselines/reference/submodule/compiler/gettersAndSettersErrors.types	Updates baseline types to reflect accessor-preferred type and TS2717 reporting.
testdata/baselines/reference/submodule/compiler/gettersAndSettersErrors.symbols	Updates baseline symbols to include merged declarations for Foo.
testdata/baselines/reference/submodule/compiler/gettersAndSettersErrors.errors.txt	Updates baseline errors to include TS2717 and revised total.
testdata/baselines/reference/submodule/compiler/fieldAndGetterWithSameName.symbols.diff	Adds new diff baseline showing merged symbol output.
testdata/baselines/reference/submodule/compiler/fieldAndGetterWithSameName.symbols	Updates baseline symbols for merged field/getter.
testdata/baselines/reference/submodule/compiler/duplicateClassElements.types	Updates baseline types for merged property/accessor inference.
testdata/baselines/reference/submodule/compiler/duplicateClassElements.symbols	Updates baseline symbols for merged declarations.
testdata/baselines/reference/submodule/compiler/duplicateClassElements.errors.txt	Updates baseline errors to include TS2717 and revised total.
testdata/baselines/reference/submodule/compiler/allowJscheckJsTypeParameterNoCrash.types.diff	Adds new diff baseline reflecting corrected watch member typing.
testdata/baselines/reference/submodule/compiler/allowJscheckJsTypeParameterNoCrash.types	Updates baseline types for watch.data2 shape.
testdata/baselines/reference/submodule/compiler/allowJscheckJsTypeParameterNoCrash.symbols	Updates baseline symbols for data2 declaration attribution.
testdata/baselines/reference/fourslash/findAllReferences/findAllRefsParameterPropertyWithConflictingMember.baseline.jsonc	Adjusts fourslash reference markers for conflicting member name handling.
testdata/baselines/reference/compiler/duplicateIdentifierChecks.types	Updates object literal inferred type now that property/accessor merging behavior changed.
testdata/baselines/reference/compiler/duplicateIdentifierChecks.symbols	Updates symbol baselines for merged declarations across accessors/properties.
testdata/baselines/reference/compiler/duplicateIdentifierChecks.errors.txt	Updates diagnostics baselines (including duplicate name formatting changes).
internal/checker/checker.go	Refines duplicate-member checking logic + prefers accessor types in getTypeOfSymbol.
internal/binder/binder.go	Adjusts binding exclusions for object-literal methods.
internal/ast/symbolflags.go	Updates exclusion masks to permit property/accessor merging and refine accessor conflicts.

Comments suppressed due to low confidence (1)

internal/checker/checker.go:1

• kind and state are currently encoded as magic integers (1/2/3), which makes the control flow hard to reason about and easy to break during future changes. Please introduce named constants (e.g., declKindProperty, declKindAccessor, declStateErrorReported) and rewrite the conditional as explicit comparisons (including parenthesizing state == 2 && kind != 2) so the intended state machine is self-documenting.

[Copilot]

Switching object-literal methods to use ast.SymbolFlagsValue as the exclude mask is a behavioral change that can affect merging between object-literal methods and same-named properties/accessors (and therefore inferred object literal types and diagnostics). Please add/extend a baseline test that covers at least { a() {}, get a() { ... } } and { a() {}, a: 1 } to ensure the binder+checker combination still produces the intended error codes and inferred member type.

Suggested change

	b.bindPropertyOrMethodOrAccessor(node, ast.SymbolFlagsMethod|getOptionalSymbolFlagForNode(node), core.IfElse(ast.IsObjectLiteralMethod(node), ast.SymbolFlagsValue, ast.SymbolFlagsMethodExcludes))
	b.bindPropertyOrMethodOrAccessor(node, ast.SymbolFlagsMethod|getOptionalSymbolFlagForNode(node), ast.SymbolFlagsMethodExcludes)

[ahejlsberg]

The tests already show the effects of this change (i.e. method names must be unique in object literals).

[Copilot]

These exclusion-mask changes are subtle and directly impact binding/merging semantics (especially around property/accessor merging being allowed in the binder and rejected later in the checker). Please add a short comment near these constants explaining the rationale and the intended invariants (e.g., “binder allows property+accessor merge; checker reports duplicates within the same declaration”), so future modifications don’t inadvertently revert the intended behavior.

Suggested change

	SymbolFlagsParameterExcludes = SymbolFlagsValue
	SymbolFlagsParameterExcludes = SymbolFlagsValue
	// Binder intentionally allows property+accessor merges by not excluding Property/Accessor
	// here; duplicate/conflicting declarations within the same declaration are reported later
	// by the checker. Keep these masks aligned so future changes do not tighten binder merging.

[Copilot]

These exclusion-mask changes are subtle and directly impact binding/merging semantics (especially around property/accessor merging being allowed in the binder and rejected later in the checker). Please add a short comment near these constants explaining the rationale and the intended invariants (e.g., “binder allows property+accessor merge; checker reports duplicates within the same declaration”), so future modifications don’t inadvertently revert the intended behavior.

Suggested change

	SymbolFlagsMethodExcludes = SymbolFlagsValue & ^SymbolFlagsMethod
	SymbolFlagsMethodExcludes = SymbolFlagsValue & ^SymbolFlagsMethod
	// Binder intentionally allows property+accessor merges by excluding accessors from
	// other value declarations but not from properties. The checker later reports duplicate
	// or otherwise invalid combinations within the same declaration.

[jakebailey]

For completeness, do we also have a test which checks this in JS files, which may not be checked?

[ahejlsberg]

Not quite sure what you mean. Nothing special about JS here.

[jakebailey]

I mean specifically that if more stuff goes into the checker, how is checkJs=false affected, if before it was a binder error.

[jakebailey]

Probably the new diffs can be marked as accepted, since they are intentional?

[jakebailey]

Within what's in the PR I think this looks fine, but I think it'd be worth checking that this all works in allowJs without checkJs