Skip to content

Commit

Permalink
feat(jsii): check that referenced @params exist (#431)
Browse files Browse the repository at this point in the history
Verify integrity of the documentation by forcing the parameter names
referred to in @param declarations to actually exist. 

Right now it's a WARNING, will be turned into an ERROR some time
in the future. Scrub your sources!

Fixes #422.
  • Loading branch information
rix0rrr authored Apr 4, 2019
1 parent afbabff commit 265c304
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 12 deletions.
30 changes: 19 additions & 11 deletions packages/jsii/lib/assembler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import log4js = require('log4js');
import path = require('path');
import ts = require('typescript');
import { JSII_DIAGNOSTICS_CODE } from './compiler';
import { parseSymbolDocumentation } from './docs';
import { getReferencedDocParams, parseSymbolDocumentation } from './docs';
import { Diagnostic, Emitter } from './emitter';
import literate = require('./literate');
import { ProjectInfo } from './project-info';
Expand Down Expand Up @@ -645,18 +645,10 @@ export class Assembler implements Emitter {
}

/**
* Register documentations on a ``spec.Documentable`` entry.
*
* @param sym the symbol holding the JSDoc information
* @param documentable the entity being documented
*
* @returns ``documentable``
* Return docs for a symbol
*/
private _visitDocumentation(sym: ts.Symbol): spec.Docs | undefined {
const comment = ts.displayPartsToString(sym.getDocumentationComment(this._typeChecker)).trim();

// Right here we'll just guess that the first declaration site is the most important one.
const result = parseSymbolDocumentation(comment, sym.getJsDocTags());
const result = parseSymbolDocumentation(sym, this._typeChecker);

for (const diag of result.diagnostics || []) {
this._diagnostic(sym.declarations[0],
Expand All @@ -669,6 +661,20 @@ export class Assembler implements Emitter {
return !allUndefined ? result.docs : undefined;
}

/**
* Check that all parameters the doc block refers to with a @param declaration actually exist
*/
private _validateReferencedDocParams(method: spec.Method, methodSym: ts.Symbol) {
const params = getReferencedDocParams(methodSym);
const actualNames = new Set((method.parameters || []).map(p => p.name));
for (const param of params) {
if (!actualNames.has(param)) {
this._diagnostic(methodSym.valueDeclaration, ts.DiagnosticCategory.Warning,
`In doc block of '${method.name}', '@param ${param}' refers to a nonexistent parameter.`);
}
}
}

private async _visitInterface(type: ts.Type, namespace: string[]): Promise<spec.InterfaceType | undefined> {
if (LOG.isTraceEnabled()) {
LOG.trace(`Processing interface: ${colors.gray(namespace.join('.'))}.${colors.cyan(type.symbol.name)}`);
Expand Down Expand Up @@ -846,6 +852,8 @@ export class Assembler implements Emitter {
});
}

this._validateReferencedDocParams(method, symbol);

type.methods = type.methods || [];
if (type.methods.find(m => m.name === method.name && m.static === method.static) != null) {
LOG.trace(`Dropping re-declaration of ${colors.green(type.fqn)}#${colors.cyan(method.name!)}`);
Expand Down
24 changes: 23 additions & 1 deletion packages/jsii/lib/docs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,29 @@
import spec = require('jsii-spec');
import ts = require('typescript');

export function parseSymbolDocumentation(comments: string | undefined, tags: ts.JSDocTagInfo[]): DocsParsingResult {
export function parseSymbolDocumentation(sym: ts.Symbol, typeChecker: ts.TypeChecker): DocsParsingResult {
const comment = ts.displayPartsToString(sym.getDocumentationComment(typeChecker)).trim();
const tags = sym.getJsDocTags();

// Right here we'll just guess that the first declaration site is the most important one.
return parseDocParts(comment, tags);
}

/**
* Return the list of parameter names that are referenced in the docstring for this symbol
*/
export function getReferencedDocParams(sym: ts.Symbol): string[] {
const ret = new Array<string>();
for (const tag of sym.getJsDocTags()) {
if (tag.name === 'param') {
const parts = (tag.text || '').split(' ');
ret.push(parts[0]);
}
}
return ret;
}

function parseDocParts(comments: string | undefined, tags: ts.JSDocTagInfo[]): DocsParsingResult {
const diagnostics = new Array<string>();
const docs: spec.Docs = {};

Expand Down

0 comments on commit 265c304

Please sign in to comment.