diff --git a/gh-pages/content/user-guides/lib-author/.pages.yml b/gh-pages/content/user-guides/lib-author/.pages.yml index 50a3e1cb10..440b1bb61b 100644 --- a/gh-pages/content/user-guides/lib-author/.pages.yml +++ b/gh-pages/content/user-guides/lib-author/.pages.yml @@ -5,4 +5,5 @@ nav: - quick-start - typescript-restrictions.md - configuration + - hints.md - toolchain diff --git a/gh-pages/content/user-guides/lib-author/hints.md b/gh-pages/content/user-guides/lib-author/hints.md new file mode 100644 index 0000000000..0781832848 --- /dev/null +++ b/gh-pages/content/user-guides/lib-author/hints.md @@ -0,0 +1,27 @@ +# Type system hints + +The `jsii` compiler interprets some documentation tags as hints that influence +the type system represented in the `.jsii` assembly files. + +## Forcing an interface to be considered a *struct* + +Using the `@struct` tag, an interface will be interpreted as a +[*struct*][struct] even if its name starts with a capital `I`, followed by +another capital letter (which normally would make them be treated as +[*behavioral interfaces*][interface]): + +[struct]: ../../specification/2-type-system.md#structs +[interface]: ../../specification/2-type-system.md#behavioral-interfaces + +```ts +/** + * @struct + */ +export interface IPRange { + readonly cidr: string: +} +``` + +!!! important + The `@struct` hint can only be used on interface declarations. Attempting to + use them on any other declaration will result in a compilation error. diff --git a/gh-pages/content/user-guides/lib-author/typescript-restrictions.md b/gh-pages/content/user-guides/lib-author/typescript-restrictions.md index 499828a201..7ae8b6fc9e 100644 --- a/gh-pages/content/user-guides/lib-author/typescript-restrictions.md +++ b/gh-pages/content/user-guides/lib-author/typescript-restrictions.md @@ -58,6 +58,12 @@ The `jsii` type model distinguishes two kinds of *interfaces*: A name convention is used to distinguish between these two: *behavioral interfaces* must have a name that starts with a `I` prefix, while *structs* must not have such a prefix. +!!! info + The [`/** @struct */` type system hint][hint] can be used to force an *interface* with a name starting with the `I` + prefix to be handled as a *struct* instead of a *behavioral interface*. + + [hint]: hints.md#forcing-an-interface-to-be-considered-a-struct + ```ts hl_lines="5-8" /** * Since there is no `I` prefix, Foo is considered to be a struct. diff --git a/packages/jsii/lib/assembler.ts b/packages/jsii/lib/assembler.ts index 26698aea5f..ba4b208faf 100644 --- a/packages/jsii/lib/assembler.ts +++ b/packages/jsii/lib/assembler.ts @@ -13,6 +13,7 @@ import { getReferencedDocParams, parseSymbolDocumentation, renderSymbolDocumentation, + TypeSystemHints, } from './docs'; import { Emitter } from './emitter'; import { JsiiDiagnostic } from './jsii-diagnostic'; @@ -1162,7 +1163,7 @@ export class Assembler implements Emitter { name: type.symbol.name, namespace: ctx.namespace.length > 0 ? ctx.namespace.join('.') : undefined, - docs: this._visitDocumentation(type.symbol, ctx), + docs: this._visitDocumentation(type.symbol, ctx).docs, }, type.symbol.valueDeclaration as ts.ClassDeclaration, ); @@ -1436,7 +1437,7 @@ export class Assembler implements Emitter { jsiiType.initializer.docs = this._visitDocumentation( constructor, memberEmitContext, - ); + ).docs; this.overrideDocComment( constructor, jsiiType.initializer.docs, @@ -1674,7 +1675,7 @@ export class Assembler implements Emitter { ); } - const docs = this._visitDocumentation(symbol, ctx); + const { docs } = this._visitDocumentation(symbol, ctx); const typeContext = ctx.replaceStability(docs?.stability); const members = type.isUnion() ? type.types : [type]; @@ -1687,7 +1688,7 @@ export class Assembler implements Emitter { }`, kind: spec.TypeKind.Enum, members: members.map((m) => { - const docs = this._visitDocumentation(m.symbol, typeContext); + const { docs } = this._visitDocumentation(m.symbol, typeContext); this.overrideDocComment(m.symbol, docs); return { name: m.symbol.name, docs }; }), @@ -1710,7 +1711,7 @@ export class Assembler implements Emitter { private _visitDocumentation( sym: ts.Symbol, context: EmitContext, - ): spec.Docs | undefined { + ): { readonly docs?: spec.Docs; readonly hints: TypeSystemHints } { const result = parseSymbolDocumentation(sym, this._typeChecker); for (const diag of result.diagnostics ?? []) { @@ -1722,6 +1723,25 @@ export class Assembler implements Emitter { ); } + const decl = sym.valueDeclaration ?? sym.declarations[0]; + // The @struct hint is only valid for interface declarations + if (!ts.isInterfaceDeclaration(decl) && result.hints.struct) { + this._diagnostics.push( + JsiiDiagnostic.JSII_7001_ILLEGAL_HINT.create( + _findHint(decl, 'struct')!, + 'struct', + 'interfaces with only readonly properties', + ) + .addRelatedInformation( + ts.getNameOfDeclaration(decl) ?? decl, + 'The annotated declaration is here', + ) + .preformat(this.projectInfo.projectRoot), + ); + // Clean up the bad hint... + delete (result.hints as any).struct; + } + // Apply the current context's stability if none was specified locally. if (result.docs.stability == null) { result.docs.stability = context.stability; @@ -1730,7 +1750,10 @@ export class Assembler implements Emitter { const allUndefined = Object.values(result.docs).every( (v) => v === undefined, ); - return !allUndefined ? result.docs : undefined; + return { + docs: !allUndefined ? result.docs : undefined, + hints: result.hints, + }; } /** @@ -1777,6 +1800,7 @@ export class Assembler implements Emitter { type.symbol.name }`; + const { docs, hints } = this._visitDocumentation(type.symbol, ctx); const jsiiType: spec.InterfaceType = bindings.setInterfaceRelatedNode( { assembly: this.projectInfo.name, @@ -1785,7 +1809,7 @@ export class Assembler implements Emitter { name: type.symbol.name, namespace: ctx.namespace.length > 0 ? ctx.namespace.join('.') : undefined, - docs: this._visitDocumentation(type.symbol, ctx), + docs, }, type.symbol.declarations[0] as ts.InterfaceDeclaration, ); @@ -1862,6 +1886,30 @@ export class Assembler implements Emitter { (...bases: spec.Type[]) => { if ((jsiiType.methods ?? []).length === 0) { jsiiType.datatype = true; + } else if (hints.struct) { + this._diagnostics.push( + jsiiType.methods!.reduce( + (diag, mthod) => { + const node = bindings.getMethodRelatedNode(mthod); + return node + ? diag.addRelatedInformation( + ts.getNameOfDeclaration(node) ?? node, + `A method is declared here`, + ) + : diag; + }, + JsiiDiagnostic.JSII_7001_ILLEGAL_HINT.create( + _findHint(declaration, 'struct')!, + 'struct', + 'interfaces with only readonly properties', + ) + .addRelatedInformation( + ts.getNameOfDeclaration(declaration) ?? declaration, + 'The annotated declartion is here', + ) + .preformat(this.projectInfo.projectRoot), + ), + ); } for (const base of bases) { @@ -1882,8 +1930,9 @@ export class Assembler implements Emitter { ); } - // If the name starts with an "I" it is not intended as a datatype, so switch that off. - if (jsiiType.datatype && interfaceName) { + // If the name starts with an "I" it is not intended as a datatype, so switch that off, + // unless a TSDoc hint was set to force this to be considered a behavioral interface. + if (jsiiType.datatype && interfaceName && !hints.struct) { delete jsiiType.datatype; } @@ -2051,7 +2100,7 @@ export class Assembler implements Emitter { this._verifyConsecutiveOptionals(declaration, method.parameters); - method.docs = this._visitDocumentation(symbol, ctx); + method.docs = this._visitDocumentation(symbol, ctx).docs; // If the last parameter is a datatype, verify that it does not share any field names with // other function arguments, so that it can be turned into keyword arguments by jsii frontends @@ -2218,7 +2267,7 @@ export class Assembler implements Emitter { property.const = true; } - property.docs = this._visitDocumentation(symbol, ctx); + property.docs = this._visitDocumentation(symbol, ctx).docs; type.properties = type.properties ?? []; if ( @@ -2273,8 +2322,8 @@ export class Assembler implements Emitter { parameter.docs = this._visitDocumentation( paramSymbol, - ctx.removeStability(), - ); // No inheritance on purpose + ctx.removeStability(), // No inheritance on purpose + ).docs; // Don't rewrite doc comment here on purpose -- instead, we add them as '@param' // into the parent's doc comment. @@ -3198,6 +3247,17 @@ function _nameOrDeclarationNode(symbol: ts.Symbol): ts.Node { return ts.getNameOfDeclaration(declaration) ?? declaration; } +function _findHint( + decl: ts.Declaration, + hint: string, +): ts.JSDocTag | undefined { + const [node] = ts.getAllJSDocTags( + decl, + (tag): tag is ts.JSDocTag => tag.tagName.text === hint, + ); + return node; +} + /** * A location where a type can be used. */ diff --git a/packages/jsii/lib/docs.ts b/packages/jsii/lib/docs.ts index aaa7ca511f..05bff439a1 100644 --- a/packages/jsii/lib/docs.ts +++ b/packages/jsii/lib/docs.ts @@ -49,6 +49,7 @@ enum DocTag { SUBCLASSABLE = 'subclassable', EXAMPLE = 'example', STABILITY = 'stability', + STRUCT = 'struct', } /** @@ -151,6 +152,7 @@ function parseDocParts( ): DocsParsingResult { const diagnostics = new Array(); const docs: spec.Docs = {}; + const hints: TypeSystemHints = {}; [docs.summary, docs.remarks] = splitSummary(comments); @@ -173,6 +175,10 @@ function parseDocParts( return undefined; } + if (eatTag(DocTag.STRUCT) != null) { + hints.struct = true; + } + docs.default = eatTag(DocTag.DEFAULT, DocTag.DEFAULT_VALUE); docs.deprecated = eatTag(DocTag.DEPRECATED); docs.example = eatTag(DocTag.EXAMPLE); @@ -233,14 +239,23 @@ function parseDocParts( } } - return { docs, diagnostics }; + return { docs, diagnostics, hints }; } export interface DocsParsingResult { docs: spec.Docs; + hints: TypeSystemHints; diagnostics?: string[]; } +export interface TypeSystemHints { + /** + * Only present on interfaces. This indicates that interface must be handled as a struct/data type + * even through it's name starts with a capital letter `I`. + */ + struct?: boolean; +} + /** * Split the doc comment into summary and remarks * diff --git a/packages/jsii/lib/jsii-diagnostic.ts b/packages/jsii/lib/jsii-diagnostic.ts index b7fbbfb0de..bf4b2bf407 100644 --- a/packages/jsii/lib/jsii-diagnostic.ts +++ b/packages/jsii/lib/jsii-diagnostic.ts @@ -2,7 +2,8 @@ import * as spec from '@jsii/spec'; import { camel, constant as allCaps, pascal } from 'case'; import * as ts from 'typescript'; -import { JSII_DIAGNOSTICS_CODE } from './utils'; +import { TypeSystemHints } from './docs'; +import { JSII_DIAGNOSTICS_CODE, _formatDiagnostic } from './utils'; /** * Descriptors for all valid jsii diagnostic codes. @@ -625,6 +626,15 @@ export class JsiiDiagnostic implements ts.Diagnostic { name: 'documentation/non-existent-parameter', }); + public static readonly JSII_7001_ILLEGAL_HINT = Code.error({ + code: 7001, + formatter: (hint: keyof TypeSystemHints, ...valid: readonly string[]) => + `Illegal use of "@${hint}" hint. It is only valid on ${valid.join( + ', ', + )}.`, + name: 'documentation/illegal-hint', + }); + public static readonly JSII_7999_DOCUMENTATION_ERROR = Code.error({ code: 7999, formatter: (messageText) => messageText, @@ -789,6 +799,9 @@ export class JsiiDiagnostic implements ts.Diagnostic { public readonly relatedInformation = new Array(); + // eslint-disable-next-line @typescript-eslint/explicit-member-accessibility + #formatted?: string; + /** * Creates a new `JsiiDiagnostic` with the provided properties. * @@ -817,6 +830,32 @@ export class JsiiDiagnostic implements ts.Diagnostic { this.relatedInformation.push( JsiiDiagnostic.JSII_9999_RELATED_INFO.create(node, message), ); + // Clearing out #formatted, as this would no longer be the correct string. + this.#formatted = undefined; + return this; + } + + /** + * Formats this diagnostic with color and context if possible, and returns it. + * The formatted diagnostic is cached, so that it can be re-used. This is + * useful for diagnostic messages involving trivia -- as the trivia may have + * been obliterated from the `SourceFile` by the `TsCommentReplacer`, which + * makes the error messages really confusing. + */ + public format(projectRoot: string): string { + if (this.#formatted == null) { + this.#formatted = _formatDiagnostic(this, projectRoot); + } + return this.#formatted; + } + + /** + * Ensures the formatted diagnostic is prepared for later re-use. + * + * @returns `this` + */ + public preformat(projectRoot: string): this { + this.format(projectRoot); return this; } } diff --git a/packages/jsii/lib/utils.ts b/packages/jsii/lib/utils.ts index af08ac0750..f413bdbf27 100644 --- a/packages/jsii/lib/utils.ts +++ b/packages/jsii/lib/utils.ts @@ -60,6 +60,27 @@ export function diagnosticsLogger( export function formatDiagnostic( diagnostic: ts.Diagnostic, projectRoot: string, +) { + if (JsiiDiagnostic.isJsiiDiagnostic(diagnostic)) { + // Ensure we leverage pre-rendered diagnostics where available. + return diagnostic.format(projectRoot); + } + return _formatDiagnostic(diagnostic, projectRoot); +} + +/** + * Formats a diagnostic message with color and context, if possible. Users + * should use `formatDiagnostic` instead, as this implementation is inteded for + * internal usafe only. + * + * @param diagnostic the diagnostic message ot be formatted. + * @param projectRoot the root of the TypeScript project. + * + * @returns a formatted string. + */ +export function _formatDiagnostic( + diagnostic: ts.Diagnostic, + projectRoot: string, ) { const formatDiagnosticsHost: ts.FormatDiagnosticsHost = { getCurrentDirectory: () => projectRoot, diff --git a/packages/jsii/test/__snapshots__/negatives.test.ts.snap b/packages/jsii/test/__snapshots__/negatives.test.ts.snap index f24de9dab3..3ed8b6469c 100644 --- a/packages/jsii/test/__snapshots__/negatives.test.ts.snap +++ b/packages/jsii/test/__snapshots__/negatives.test.ts.snap @@ -699,6 +699,49 @@ neg.struct-extends-interface.ts:6:18 - error JSII8007: Interface contains behavi `; +exports[`struct-hint-on-class 1`] = ` +neg.struct-hint-on-class.ts:4:4 - error JSII7001: Illegal use of "@struct" hint. It is only valid on interfaces with only readonly properties. + +4 * @struct + ~~~~~~~ + + neg.struct-hint-on-class.ts:6:14 + 6 export class ClassName { } + ~~~~~~~~~ + The annotated declaration is here + +`; + +exports[`struct-hint-on-enum 1`] = ` +neg.struct-hint-on-enum.ts:4:4 - error JSII7001: Illegal use of "@struct" hint. It is only valid on interfaces with only readonly properties. + +4 * @struct + ~~~~~~~ + + neg.struct-hint-on-enum.ts:6:13 + 6 export enum EnumName { A, B } + ~~~~~~~~ + The annotated declaration is here + +`; + +exports[`struct-hint-with-methods 1`] = ` +neg.struct-hint-with-methods.ts:1:72 - error JSII7001: Illegal use of "@struct" hint. It is only valid on interfaces with only readonly properties. + +1 + + + neg.struct-hint-with-methods.ts:2:18 + 2 export interface INotAStruct { + ~~~~~~~~~~~ + The annotated declartion is here + neg.struct-hint-with-methods.ts:3:3 + 3 method(): void; + ~~~~~~ + A method is declared here + +`; + exports[`submodules-cannot-have-colliding-names 1`] = ` neg.submodules-cannot-have-colliding-names.ts:3:14 - error JSII5011: Submodule "ns1" conflicts with "Ns1, as different languages could represent it as: "ns1", "Ns1"" diff --git a/packages/jsii/test/hints.test.ts b/packages/jsii/test/hints.test.ts new file mode 100644 index 0000000000..0df21078ca --- /dev/null +++ b/packages/jsii/test/hints.test.ts @@ -0,0 +1,33 @@ +import { InterfaceType, TypeKind } from '@jsii/spec'; + +import { sourceToAssemblyHelper } from '../lib'; + +describe('@struct', () => { + test('causes behavioral-named interfaces to be structs', async () => { + const assembly = await sourceToAssemblyHelper(` + /** @struct */ + export interface IPSet { + readonly cidr: string; + } + `); + + expect(assembly.types!['testpkg.IPSet'].kind).toBe(TypeKind.Interface); + expect((assembly.types!['testpkg.IPSet'] as InterfaceType).datatype).toBe( + true, + ); + }); + + test('can be used on any struct', async () => { + const assembly = await sourceToAssemblyHelper(` + /** @struct */ + export interface Struct { + readonly cidr: string; + } + `); + + expect(assembly.types!['testpkg.Struct'].kind).toBe(TypeKind.Interface); + expect((assembly.types!['testpkg.Struct'] as InterfaceType).datatype).toBe( + true, + ); + }); +}); diff --git a/packages/jsii/test/negatives/neg.struct-hint-on-class.ts b/packages/jsii/test/negatives/neg.struct-hint-on-class.ts new file mode 100644 index 0000000000..84c5a5d3ce --- /dev/null +++ b/packages/jsii/test/negatives/neg.struct-hint-on-class.ts @@ -0,0 +1,6 @@ +/** + * Attempting to tag a class as a struct. + * + * @struct + */ +export class ClassName { } diff --git a/packages/jsii/test/negatives/neg.struct-hint-on-enum.ts b/packages/jsii/test/negatives/neg.struct-hint-on-enum.ts new file mode 100644 index 0000000000..3d6bb6442e --- /dev/null +++ b/packages/jsii/test/negatives/neg.struct-hint-on-enum.ts @@ -0,0 +1,6 @@ +/** + * Attempting to tag an enum as a struct. + * + * @struct + */ +export enum EnumName { A, B } diff --git a/packages/jsii/test/negatives/neg.struct-hint-with-methods.ts b/packages/jsii/test/negatives/neg.struct-hint-with-methods.ts new file mode 100644 index 0000000000..0f45d46192 --- /dev/null +++ b/packages/jsii/test/negatives/neg.struct-hint-with-methods.ts @@ -0,0 +1,8 @@ +/** + * Attempt to force a behavioral interface to be a struct... + * + * @struct + */ +export interface INotAStruct { + method(): void; +}