Skip to content

Commit

Permalink
feat: Document overriden/inherited members (#238)
Browse files Browse the repository at this point in the history 
Add a documentation line outlining the parent declaration of overridden
members as well as inherited members that are not overridden locally.
This should make browsing the documentation a lot nicer.

Inherited and overridden statements in the documentation always refer to
the fully qualified name of the super statement.

Fixes #196
  • Loading branch information
@RomainMuller
RomainMuller committed Sep 19, 2018
1 parent 30d1491 commit 7a6278a
Show file tree
Hide file tree
Showing 5 changed files with 668 additions and 37 deletions.
14 changes: 10 additions & 4 deletions packages/jsii-pacmak/bin/jsii-pacmak.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { VERSION_DESC } from '../lib/version';
const targetConstructors = await Target.findAll();
const argv = yargs
.usage('Usage: jsii-pacmak [-t target,...] [-o outdir] [package-dir]')
.env('JSII_PACMAK')
.option('targets', {
alias: ['target', 't'],
type: 'array',
Expand Down Expand Up @@ -47,6 +48,11 @@ import { VERSION_DESC } from '../lib/version';
desc: 'force generation of new artifacts, even if the fingerprints match',
default: false
})
.option('force-subdirectory', {
type: 'boolean',
desc: 'force generation into a target-named subdirectory, even in single-target mode',
default: false
})
.option('recurse', {
alias: 'R',
type: 'boolean',
Expand Down Expand Up @@ -80,9 +86,9 @@ import { VERSION_DESC } from '../lib/version';
const rootDir = path.resolve(process.cwd(), argv._[0] || '.');

const visited = new Set<string>();
await buildPackage(rootDir);
await buildPackage(rootDir, true /* isRoot */, argv.forceSubdirectory);

async function buildPackage(packageDir: string, isRoot = true) {
async function buildPackage(packageDir: string, isRoot: boolean, forceSubdirectory: boolean) {
if (visited.has(packageDir)) {
return; // already built
}
Expand All @@ -103,7 +109,7 @@ import { VERSION_DESC } from '../lib/version';
if (argv.recurse) {
for (const dep of Object.keys(pkg.dependencies || { })) {
const depDir = resolveDependencyDirectory(packageDir, dep);
await buildPackage(depDir, /* isRoot */ false);
await buildPackage(depDir, /* isRoot */ false, forceSubdirectory);
}
}

Expand All @@ -127,7 +133,7 @@ import { VERSION_DESC } from '../lib/version';
const tarball = await npmPack(packageDir, tmpdir);
for (const targetName of targets) {
// if we are targeting a single language, output to outdir, otherwise outdir/<target>
const targetOutputDir = targets.length > 1 ? path.join(outDir, targetName) : outDir;
const targetOutputDir = (targets.length > 1 || forceSubdirectory) ? path.join(outDir, targetName) : outDir;
logging.debug(`Building ${pkg.name}/${targetName}: ${targetOutputDir}`);
await generateTarget(packageDir, targetName, targetOutputDir, tarball);
}
Expand Down
108 changes: 101 additions & 7 deletions packages/jsii-pacmak/lib/targets/sphinx.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,8 @@ class SphinxDocsGenerator extends Generator {
this.namespaceStack.push({ name: className, underClass: true });
}

protected onEndClass(_cls: spec.ClassType) {
protected onEndClass(cls: spec.ClassType) {
this.renderInheritedMembers(cls);
this.code.closeBlock();
this.namespaceStack.pop();
if (!this.topNamespace.underClass) { this.closeSection(); }
Expand Down Expand Up @@ -342,7 +343,8 @@ class SphinxDocsGenerator extends Generator {
this.code.line();
}

protected onEndInterface(_ifc: spec.InterfaceType) {
protected onEndInterface(ifc: spec.InterfaceType) {
this.renderInheritedMembers(ifc);
this.code.closeBlock();
if (!this.topNamespace.underClass) { this.closeSection(); }
}
Expand All @@ -359,6 +361,62 @@ class SphinxDocsGenerator extends Generator {
this.renderProperty(property);
}

private renderInheritedMembers(entity: spec.ClassType | spec.InterfaceType) {
const inherited = this.getInheritedMembers(entity);
if (Object.keys(inherited).length === 0) { return; }
for (const source of Object.keys(inherited).sort()) {
const entities = inherited[source];
for (const method of entities.methods) {
this.renderMethod(method, source);
for (const overload of this.createOverloadsForOptionals(method)) {
this.renderMethod(overload, source);
}
}
for (const property of entities.properties) {
this.renderProperty(property, source);
}
}
}

private getInheritedMembers(entity: spec.ClassType | spec.InterfaceType): InheritedMembers {
const parents = parentTypes(entity);
const knownMembers = new Set<string>([
...(entity.methods || []).map(m => m.name!),
...(entity.properties || []).map(p => p.name)
]);
const result: InheritedMembers = {};
for (const parent of parents) {
const parentType = this.findType(parent.fqn) as spec.ClassType | spec.InterfaceType;
for (const method of parentType.methods || []) {
if (method.static || knownMembers.has(method.name!)) { continue; }
result[parentType.fqn] = result[parentType.fqn] || { methods: [], properties: [] };
result[parentType.fqn].methods.push(method);
knownMembers.add(method.name!);
}
for (const property of parentType.properties || []) {
if (property.static || knownMembers.has(property.name!)) { continue; }
result[parentType.fqn] = result[parentType.fqn] || { methods: [], properties: [] };
result[parentType.fqn].properties.push(property);
knownMembers.add(property.name);
}
for (const superType of parentTypes(parentType)) {
parents.push(superType);
}
}
return result;

function parentTypes(type: spec.ClassType | spec.InterfaceType) {
const types = new Array<spec.NamedTypeReference>();
if (spec.isClassType(type) && type.base) {
types.push(type.base);
}
if (type.interfaces) {
types.push(...type.interfaces);
}
return types;
}
}

/**
* Adds a title to the current code file, using the appropriate header
* adornment given the current TOC depth. It will start using simple
Expand Down Expand Up @@ -402,7 +460,7 @@ class SphinxDocsGenerator extends Generator {
signature += ', ';
}

if (p.type.optional) {
if (p.type.optional && !params.slice(idx + 1).find(e => !e.type.optional)) {
signature += '[';
signaturePosfix += ']';
}
Expand Down Expand Up @@ -437,22 +495,40 @@ class SphinxDocsGenerator extends Generator {
}
}

private renderMethod(method: spec.Method) {
private renderMethod(method: spec.Method, inheritedFrom?: string) {
const signature = this.renderMethodSignature(method);

const type = method.static ? `py:staticmethod` : `py:method`;

this.code.line();
this.code.openBlock(`.. ${type}:: ${method.name}${signature}`);

if (inheritedFrom) {
this.code.line();
this.code.line(`*Inherited from* :py:meth:\`${inheritedFrom} <${inheritedFrom}.${method.name}>\``);
} else if (method.overrides) {
this.code.line();
const superType = this.findType(method.overrides.fqn) as spec.ClassType | spec.InterfaceType;
if (spec.isInterfaceType(superType) || superType.methods!.find(m => m.name === method.name && !!m.abstract)) {
this.code.line(`*Implements* :py:meth:\`${method.overrides.fqn}.${method.name}\``);
} else {
this.code.line(`*Overrides* :py:meth:\`${method.overrides.fqn}.${method.name}\``);
}
}
this.renderDocsLine(method);
this.code.line();
if (method.protected) {
this.code.line('*Protected method*');
this.code.line();
}

this.renderMethodParameters(method);

// @return doc
if (method.docs && method.docs.return) {
this.code.line(`:return: ${method.docs.return}`);
const [firstLine, ...rest] = method.docs.return.split('\n');
this.code.line(`:return: ${firstLine}`);
rest.forEach(line => this.code.line(` ${line}`));
}

if (method.returns) {
Expand Down Expand Up @@ -542,7 +618,7 @@ class SphinxDocsGenerator extends Generator {
} else {
throw new Error('Unexpected type ref');
}
if (type.optional) { result.ref = `${result.ref} or undefined`; }
if (type.optional) { result.ref = `${result.ref} or \`\`undefined\`\``; }
return result;

// Wrap a string between parenthesis if it contains " or "
Expand All @@ -552,12 +628,28 @@ class SphinxDocsGenerator extends Generator {
}
}

private renderProperty(prop: spec.Property) {
private renderProperty(prop: spec.Property, inheritedFrom?: string) {
this.code.line();
const type = this.renderTypeRef(prop.type);
this.code.openBlock(`.. py:attribute:: ${prop.name}`);
if (inheritedFrom) {
this.code.line();
this.code.line(`*Inherited from* :py:attr:\`${inheritedFrom} <${inheritedFrom}.${prop.name}>\``);
} else if (prop.overrides) {
this.code.line();
const superType = this.findType(prop.overrides.fqn) as spec.ClassType | spec.InterfaceType;
if (spec.isInterfaceType(superType) || superType.properties!.find(p => p.name === prop.name && !!p.abstract)) {
this.code.line(`*Implements* :py:meth:\`${prop.overrides.fqn}.${prop.name}\``);
} else {
this.code.line(`*Overrides* :py:attr:\`${prop.overrides.fqn}.${prop.name}\``);
}
}
this.renderDocsLine(prop);
this.code.line();
if (prop.protected) {
this.code.line('*Protected property*');
this.code.line();
}
const readonly = prop.immutable ? ' *(readonly)*' : '';
const abs = prop.abstract ? ' *(abstract)*' : '';
const stat = prop.static ? ' *(static)*' : '';
Expand Down Expand Up @@ -665,3 +757,5 @@ function formatLanguage(language: string): string {
return language;
}
}

type InheritedMembers = { [typeFqn: string]: { methods: spec.Method[], properties: spec.Property[] } };
7 changes: 7 additions & 0 deletions packages/jsii-pacmak/test/expected.jsii-calc-base/sphinx/_scope_jsii-calc-base.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,10 @@ BaseProps (interface)
:type: string *(abstract)*


.. py:attribute:: foo
*Inherited from* :py:attr:`@scope/jsii-calc-base-of-base.VeryBaseProps <@scope/jsii-calc-base-of-base.VeryBaseProps.foo>`

:type: :py:class:`@scope/jsii-calc-base-of-base.Very`\ *(abstract)*


56 changes: 52 additions & 4 deletions packages/jsii-pacmak/test/expected.jsii-calc-lib/sphinx/_scope_jsii-calc-lib.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ MyFirstStruct (interface)

.. py:attribute:: firstOptional
:type: string[] or undefined *(abstract)*
:type: string[] or ``undefined`` *(abstract)*


Number
Expand Down Expand Up @@ -296,12 +296,32 @@ Number

.. py:attribute:: value
*Implements* :py:meth:`@scope/jsii-calc-lib.Value.value`

The number.


:type: number *(readonly)*


.. py:method:: typeName() -> any
*Inherited from* :py:meth:`@scope/jsii-calc-base.Base <@scope/jsii-calc-base.Base.typeName>`

:return: the name of the class (to verify native type names are created for derived classes).
:rtype: any


.. py:method:: toString() -> string
*Inherited from* :py:meth:`@scope/jsii-calc-lib.Value <@scope/jsii-calc-lib.Value.toString>`

String representation of the value.


:rtype: string


Operation
^^^^^^^^^

Expand Down Expand Up @@ -337,13 +357,33 @@ Operation

.. py:method:: toString() -> string
*Overrides* :py:meth:`@scope/jsii-calc-lib.Value.toString`

String representation of the value.


:rtype: string
:abstract: Yes


.. py:method:: typeName() -> any
*Inherited from* :py:meth:`@scope/jsii-calc-base.Base <@scope/jsii-calc-base.Base.typeName>`

:return: the name of the class (to verify native type names are created for derived classes).
:rtype: any


.. py:attribute:: value
*Inherited from* :py:attr:`@scope/jsii-calc-lib.Value <@scope/jsii-calc-lib.Value.value>`

The value.


:type: number *(readonly)* *(abstract)*


StructWithOnlyOptionals (interface)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down Expand Up @@ -381,17 +421,17 @@ StructWithOnlyOptionals (interface)
The first optional!


:type: string or undefined *(abstract)*
:type: string or ``undefined`` *(abstract)*


.. py:attribute:: optional2
:type: number or undefined *(abstract)*
:type: number or ``undefined`` *(abstract)*


.. py:attribute:: optional3
:type: boolean or undefined *(abstract)*
:type: boolean or ``undefined`` *(abstract)*


Value
Expand Down Expand Up @@ -443,3 +483,11 @@ Value
:type: number *(readonly)* *(abstract)*


.. py:method:: typeName() -> any
*Inherited from* :py:meth:`@scope/jsii-calc-base.Base <@scope/jsii-calc-base.Base.typeName>`

:return: the name of the class (to verify native type names are created for derived classes).
:rtype: any


Loading

0 comments on commit 7a6278a

Please sign in to comment.