Skip to content

Commit

Permalink
feat: @struct type system hint (#2965)
Browse files Browse the repository at this point in the history
Using the hint allows any interface to always be treated as a struct (as
long as all it's properties are readonly and it has no methods) even if
it has an `I`-prefixed name.
  • Loading branch information
RomainMuller committed Aug 24, 2021
1 parent ebc6753 commit a4ed9a8
Show file tree
Hide file tree
Showing 12 changed files with 280 additions and 15 deletions.
1 change: 1 addition & 0 deletions gh-pages/content/user-guides/lib-author/.pages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@ nav:
- quick-start
- typescript-restrictions.md
- configuration
- hints.md
- toolchain
27 changes: 27 additions & 0 deletions gh-pages/content/user-guides/lib-author/hints.md
Original file line number Diff line number Diff line change
@@ -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.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
86 changes: 73 additions & 13 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
getReferencedDocParams,
parseSymbolDocumentation,
renderSymbolDocumentation,
TypeSystemHints,
} from './docs';
import { Emitter } from './emitter';
import { JsiiDiagnostic } from './jsii-diagnostic';
Expand Down Expand Up @@ -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,
);
Expand Down Expand Up @@ -1436,7 +1437,7 @@ export class Assembler implements Emitter {
jsiiType.initializer.docs = this._visitDocumentation(
constructor,
memberEmitContext,
);
).docs;
this.overrideDocComment(
constructor,
jsiiType.initializer.docs,
Expand Down Expand Up @@ -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];
Expand All @@ -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 };
}),
Expand All @@ -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 ?? []) {
Expand All @@ -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;
Expand All @@ -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,
};
}

/**
Expand Down Expand Up @@ -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,
Expand All @@ -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,
);
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
*/
Expand Down
17 changes: 16 additions & 1 deletion packages/jsii/lib/docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ enum DocTag {
SUBCLASSABLE = 'subclassable',
EXAMPLE = 'example',
STABILITY = 'stability',
STRUCT = 'struct',
}

/**
Expand Down Expand Up @@ -151,6 +152,7 @@ function parseDocParts(
): DocsParsingResult {
const diagnostics = new Array<string>();
const docs: spec.Docs = {};
const hints: TypeSystemHints = {};

[docs.summary, docs.remarks] = splitSummary(comments);

Expand All @@ -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);
Expand Down Expand Up @@ -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
*
Expand Down
41 changes: 40 additions & 1 deletion packages/jsii/lib/jsii-diagnostic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -789,6 +799,9 @@ export class JsiiDiagnostic implements ts.Diagnostic {
public readonly relatedInformation =
new Array<ts.DiagnosticRelatedInformation>();

// eslint-disable-next-line @typescript-eslint/explicit-member-accessibility
#formatted?: string;

/**
* Creates a new `JsiiDiagnostic` with the provided properties.
*
Expand Down Expand Up @@ -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;
}
}
Expand Down
21 changes: 21 additions & 0 deletions packages/jsii/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading

0 comments on commit a4ed9a8

Please sign in to comment.