Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: @struct type system hint #2965

Merged
merged 3 commits into from
Aug 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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