Adding partial keyword support to interface. - #11948
Gerardo Lecaros (glecaros) wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues affect inheritance, decorator handling, and modifier validation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds partial interface support across the compiler, tooling, formatter, and syntax highlighting.
Changes:
- Adds parsing, AST, modifier, completion, formatting, and Monarch support.
- Merges partial interfaces across files, including operations, decorators, and inheritance.
- Adds diagnostics, tests, and a changeset.
Unresolved findings include a critical crash path, incomplete cross-file inheritance merging, decorator uniqueness issues, and overly broad modifier validation.
File summaries
| File | Summary |
|---|---|
packages/monarch/test/typespec-monarch.test.ts |
Tests partial syntax highlighting. |
packages/monarch/src/typespec-monarch.ts |
Adds Monarch keyword support. |
packages/compiler/test/server/completion.test.ts |
Tests partial completion. |
packages/compiler/test/scanner.test.ts |
Tests keyword tokenization. |
packages/compiler/test/parser.test.ts |
Tests parsing and modifier usage. |
packages/compiler/test/formatter/scenarios/outputs/interface.tsp |
Adds expected formatter output. |
packages/compiler/test/formatter/scenarios/inputs/interface.tsp |
Adds formatter input coverage. |
packages/compiler/test/checker/interface.test.ts |
Tests partial-interface checking. |
packages/compiler/src/server/completion.ts |
Adds completion support. |
packages/compiler/src/formatter/print/printer.ts |
Formats partial. |
packages/compiler/src/core/types.ts |
Adds syntax and modifier types. |
packages/compiler/src/core/scanner.ts |
Defines the keyword token. |
packages/compiler/src/core/parser.ts |
Parses the modifier. |
packages/compiler/src/core/name-resolver.ts |
Merges cross-file symbols. |
packages/compiler/src/core/modifiers.ts |
Registers modifier compatibility. |
packages/compiler/src/core/messages.ts |
Adds diagnostics. |
packages/compiler/src/core/decorator-utils.ts |
Handles decorator uniqueness. |
packages/compiler/src/core/checker.ts |
Checks merged interfaces. |
packages/compiler/src/core/binder.ts |
Binds partial declarations. |
.chronus/changes/partial-interface-2026-9-9-19-30-0.md |
Documents the feature. |
Review details
Suppressed comments (1)
packages/compiler/src/core/decorator-utils.ts:320
ownerNodesnow includes every declaration on the symbol, so this changesvalidateDecoratorUniqueOnNodefor existing merged namespaces as well as partial interfaces. For example, two namespace declarations with one@doceach now satisfysameDecorators.length > 1and reportduplicate-decorator, even thoughinitializeTypeForNamespaceintentionally applies decorators from each namespace declaration separately (checker.ts:2513-2518). Restrict the cross-declaration lookup to partial interfaces or preserve the previous behavior for other merged symbols.
const ownerNodes: readonly Node[] = type.node?.symbol
? type.node.symbol.declarations
: type.node
? [type.node]
- Files reviewed: 20/20 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
commit: |
|
All changed packages have been documented.
Show changes
|
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate findings remain in syntax-kind stability, modifier propagation, diagnostics, template validation, and decorator handling.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (5)
packages/compiler/src/core/binder.ts:644
- The added tests cover root-level partial interfaces, but not the new
declareNamespaceMemberpath at this merge hook. A regression for partial interfaces nested in a named namespace—especially cross-file merging through recursive namespace export tables—would pass all current tests; add same-file and cross-file namespace cases that assert the merged operations and mismatch diagnostics.
if (
flags & SymbolFlags.Interface &&
mergePartialInterfaceDeclarations(node as InterfaceStatementNode, scope)
) {
packages/compiler/src/core/binder.ts:724
- Because
existingIsPartialis false for an ordinary interface, this condition also reportspartial-interface-mismatchforinterface Foo {}; interface Foo {};. That diagnostic is only for a mixed partial/non-partial set; let the all-non-partial case use the existing duplicate-symbol path.
if (!isPartial || !existingIsPartial) {
packages/compiler/src/core/checker.ts:7818
- The merged-declaration validation is after the
links.declaredTypeearly return. Ifpartial interface Foo {}is declared first and a later partial declaration has template parameters, the first check caches the type and the later call returns before this loop, so the requiredpartial-interface-templatediagnostic is missed. Validate all declaration template parameters before the cached-type return.
for (const declNode of declarations) {
checkModifiers(program, declNode);
if (
declNode.modifierFlags & ModifierFlags.Partial &&
declNode.templateParameters.length > 0
) {
packages/compiler/src/core/name-resolver.ts:1310
- This
elsebranch is also selected when bothsourceBindingandtargetBindingcontain only non-partial interfaces, so duplicate interface declarations across files gain an incorrectpartial-interface-mismatchin addition toduplicate-symbol. Only report the new diagnostic when at least one side contains a partial declaration; otherwise preserve normal duplicate handling.
} else {
program.reportDiagnostic(
createDiagnostic({
code: "partial-interface-mismatch",
format: { name: key },
target: sourceBinding.declarations[0] ?? getSymNode(sourceBinding),
packages/compiler/test/checker/interface.test.ts:479
- The added cross-file test only merges top-level
Foo; it does not exercise the recursive namespace path inname-resolver.ts:1222-1226, even though the PR promises partial interfaces in the same namespace across files. Add a two-file test with both declarations inside a namespace so a regression in recursive symbol-table merging is caught.
it("combines operations from multiple partial declarations across files", async () => {
const [{ Foo }, diagnostics] = await Tester.files({
"other.tsp": `
partial interface Foo {
b(): void;
}
`,
}).compileAndDiagnose(t.code`
import "./other.tsp";
partial interface ${t.interface("Foo")} {
a(): void;
}
`);
expectDiagnosticEmpty(diagnostics);
deepStrictEqual([...Foo.operations.keys()].sort(), ["a", "b"]);
- Files reviewed: 20/20 changed files
- Comments generated: 4
- Review effort level: Lite
| InternalKeyword, | ||
| AutoKeyword, | ||
| FunctionTypeExpression, | ||
| PartialKeyword, |
| // we have an existing binding, so just push this node to its declarations | ||
| mutate(existingBinding.declarations).push(node); | ||
| mutate(node).symbol = existingBinding; |
| const ownerNodes: readonly Node[] = type.node?.symbol | ||
| ? type.node.symbol.declarations | ||
| : type.node | ||
| ? [type.node] | ||
| : []; |
| if (allDeclarationsArePartial(sourceBinding) && allDeclarationsArePartial(targetBinding)) { | ||
| mergedSymbols.set(sourceBinding, targetBinding); | ||
| mutate(targetBinding.declarations).push(...sourceBinding.declarations); | ||
| // Combine the operations declared in each partial declaration into a single |
|
You can try these changes here
|
| - "@typespec/compiler" | ||
| --- | ||
|
|
||
| Add support for `partial` interfaces. A `partial interface` can be declared multiple times, including across different files, and every matching declaration must be marked `partial`. All operations, decorators, and `extends` clauses from each declaration are combined into a single interface. |
There was a problem hiding this comment.
Not sure this is something we really want to go towards, from the experience of TypeSpec merging namespaces has been quite an issue we wished we didn't do and expanding that to other types will make everything even more complex, make any kind of incremental compilation impossible or way harder.
TypeSpec is more built around composition of mixins(with spread, extends for interfaces) and quite reluctant of going down the c# path for this.
Summary
Add a
partialmodifier forinterfacedeclarations so that a single logical interface can besplit across multiple declarations — in the same file or across files — and have them combined
into one
Interfacetype during compilation, as long as every declaration of that interface ismarked
partial.This is a common pattern in other languages (e.g. C#'s
partial class) and is useful in TypeSpecfor splitting large interfaces across files, letting generated/scaffolded operations live alongside
hand-authored ones, or letting multiple libraries/specs contribute operations to a shared interface.
Motivating example
Both declarations combine into a single
Widgetsinterface with all four operations, decorators,and
extendsclauses merged.Proposed behavior
partialis a new contextual modifier keyword, valid only oninterfacedeclarations (alongsidethe existing
internalmodifier). Using it on any other declaration kind is a compile error.partial, all declarations of thatname (same file or across files) must be
partial. Mixing partial and non-partial declarations ofthe same interface name produces a clear diagnostic (
partial-interface-mismatch) rather than ageneric
duplicate-symbolerror.so
partial interface Foodeclared in two different.tspfiles in the same namespace stillmerges into one
Interfacetype.declarations that redeclare the same operation name still produce the existing
interface-duplicatediagnostic — partial declarations don't relax that check.extendsclauses from every partial declaration are combined.@doc("...")) on each partial declaration apply independently, exactly aswritten — this matches normal TypeSpec decorator semantics (last decorator processed wins for
decorators like
@doc), and lets each declaration legitimately contribute its own decorators.@@dec(Foo, ...)) and the doc-comment-derived decorator are applied to themerged interface exactly once, regardless of how many partial declarations exist, since they
target the shared underlying symbol rather than a specific declaration node.
validateDecoratorUniqueOnNodehelper, e.g.@doc) correctly detect a duplicate even when the twoapplications are on two different partial declarations of the same interface — consistent with
writing the decorator twice on a single non-partial declaration.
partial interface Foo<T> { ... }(templated partial interfaces) is rejected with a dedicateddiagnostic (
partial-interface-template); templates aren't supported for the POC.partial interface Foo { ... }(with no other declarations to merge with) isallowed and behaves like a normal interface.
Scope of this change / POC
This is a proof-of-concept limited to
interfacedeclarations. It touches:partialkeyword token and modifier parsing.tables) so operations from every file are visible together.
extends, and operations across all partialdeclarations when building the
Interfacetype; new diagnostics for mismatched/templated partialdeclarations.
Not yet explored:
partialonmodel,namespacere-opening semantics beyond what already exists,or any interaction with template instantiation.
Testing
The POC includes unit test coverage for: same-file and cross-file merging, decorator combination
and deduplication (augment decorators, doc comments, and per-declaration inline decorators),
extendscombination, duplicate-operation-name diagnostics, mismatched-partial diagnostics (samefile and cross-file), the templated-partial-interface diagnostic, the duplicate-decorator diagnostic
across partial declarations, and rejection of
partialon non-interface declarations. Parser,scanner, completion, and monarch (syntax highlighting) tests were also added/updated.