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

[labs/analyzer] Fix support for static class members. #3648

Merged
merged 4 commits into from
Feb 7, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/lazy-rats-hug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lit-labs/analyzer': minor
---

Fix support for static class members by storing them in separate maps by name.
14 changes: 10 additions & 4 deletions packages/labs/analyzer/src/lib/javascript/classes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,22 +72,26 @@ export const getClassMembers = (
analyzer: AnalyzerInterface
) => {
const fieldMap = new Map<string, ClassField>();
const staticFieldMap = new Map<string, ClassField>();
const methodMap = new Map<string, ClassMethod>();
const staticMethodMap = new Map<string, ClassMethod>();
declaration.members.forEach((node) => {
if (ts.isMethodDeclaration(node)) {
methodMap.set(
const info = getMemberInfo(node);
(info.static ? staticMethodMap : methodMap).set(
node.name.getText(),
new ClassMethod({
...getMemberInfo(node),
...info,
...getFunctionLikeInfo(node, analyzer),
...parseNodeJSDocInfo(node),
})
);
} else if (ts.isPropertyDeclaration(node)) {
fieldMap.set(
const info = getMemberInfo(node);
(info.static ? staticFieldMap : fieldMap).set(
node.name.getText(),
new ClassField({
...getMemberInfo(node),
...info,
default: node.initializer?.getText(),
type: getTypeForNode(node, analyzer),
...parseNodeJSDocInfo(node),
Expand All @@ -97,7 +101,9 @@ export const getClassMembers = (
});
return {
fieldMap,
staticFieldMap,
methodMap,
staticMethodMap,
};
};

Expand Down
38 changes: 30 additions & 8 deletions packages/labs/analyzer/src/lib/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -407,22 +407,28 @@ export interface ClassDeclarationInit extends DeclarationInit {
node: ts.ClassDeclaration;
getHeritage: () => ClassHeritage;
fieldMap?: Map<string, ClassField> | undefined;
staticFieldMap?: Map<string, ClassField> | undefined;
methodMap?: Map<string, ClassMethod> | undefined;
staticMethodMap?: Map<string, ClassMethod> | undefined;
}

export class ClassDeclaration extends Declaration {
readonly node: ts.ClassDeclaration;
private _getHeritage: () => ClassHeritage;
private _heritage: ClassHeritage | undefined = undefined;
readonly _fieldMap: Map<string, ClassField>;
readonly _staticFieldMap: Map<string, ClassField>;
readonly _methodMap: Map<string, ClassMethod>;
readonly _staticMethodMap: Map<string, ClassMethod>;

constructor(init: ClassDeclarationInit) {
super(init);
this.node = init.node;
this._getHeritage = init.getHeritage;
this._fieldMap = init.fieldMap ?? new Map();
this._staticFieldMap = init.staticFieldMap ?? new Map();
this._methodMap = init.methodMap ?? new Map();
this._staticMethodMap = init.staticMethodMap ?? new Map();
}

/**
Expand All @@ -434,35 +440,51 @@ export class ClassDeclaration extends Declaration {
}

/**
* Returns iterator of the `ClassField`s defined on the immediate class
* (excluding any inherited members).
* Returns iterator of the non-static `ClassField`s defined on the immediate
* class (excluding any inherited members).
*/
get fields() {
get fields(): IterableIterator<ClassField> {
return this._fieldMap.values();
}

/**
* Returns iterator of the `ClassMethod`s defined on the immediate class
* Returns iterator of the static `ClassField`s defined on the immediate class
* (excluding any inherited members).
*/
get staticFields(): IterableIterator<ClassField> {
return this._staticFieldMap.values();
}

/**
* Returns iterator of the non-static `ClassMethod`s defined on the immediate
* class (excluding any inherited members).
*/
get methods(): IterableIterator<ClassMethod> {
return this._methodMap.values();
}

/**
* Returns iterator of the static `ClassMethod`s defined on the immediate
* class (excluding any inherited members).
*/
get staticMethods(): IterableIterator<ClassMethod> {
return this._staticMethodMap.values();
}

/**
* Returns a `ClassField` model the given name defined on the immediate class
* (excluding any inherited members).
*/
getField(name: string): ClassField | undefined {
return this._fieldMap.get(name);
getField(name: string, isStatic = false): ClassField | undefined {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we have fields and staticFields, I'm not sure what value there is in a parameter to separate the two here. I would default to mirroring the two APIs and making getStaticField(), etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sgtm, done

return (isStatic ? this._staticFieldMap : this._fieldMap).get(name);
}

/**
* Returns a `ClassMethod` model for the given name defined on the immediate
* class (excluding any inherited members).
*/
getMethod(name: string): ClassMethod | undefined {
return this._methodMap.get(name);
getMethod(name: string, isStatic = false): ClassMethod | undefined {
return (isStatic ? this._staticMethodMap : this._methodMap).get(name);
}

/**
Expand Down
47 changes: 47 additions & 0 deletions packages/labs/analyzer/src/test/lit-element/jsdoc_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,18 @@ nisi ut aliquip ex ea commodo consequat.`
assert.equal(member.deprecated, 'Class field 4 deprecated');
});

test('static field1', ({getModule}) => {
const element = getModule('element-a').getDeclaration('ElementA');
assert.ok(element.isClassDeclaration());
const member = element.getField('field1', true);
assert.ok(member?.isClassField());
assert.equal(member.summary, `Static class field 1 summary`);
assert.equal(member.description, `Static class field 1 description`);
assert.equal(member.default, undefined);
assert.equal(member.privacy, 'protected');
assert.equal(member.type?.text, 'string | number');
});

// Methods

test('method1', ({getModule}) => {
Expand Down Expand Up @@ -331,6 +343,41 @@ nisi ut aliquip ex ea commodo consequat.`
assert.equal(member.deprecated, 'Method 2 deprecated');
});

test('static method1', ({getModule}) => {
const element = getModule('element-a').getDeclaration('ElementA');
assert.ok(element.isClassDeclaration());
const member = element.getMethod('method1', true);
assert.ok(member?.isClassMethod());
assert.equal(member.summary, `Static method 1 summary`);
assert.equal(member.description, `Static method 1 description`);
assert.equal(member.parameters?.length, 3);
assert.equal(member.parameters?.[0].name, 'a');
assert.equal(member.parameters?.[0].description, 'Param a description');
assert.equal(member.parameters?.[0].summary, undefined);
assert.equal(member.parameters?.[0].type?.text, 'string');
assert.equal(member.parameters?.[0].default, undefined);
assert.equal(member.parameters?.[0].rest, false);
assert.equal(member.parameters?.[1].name, 'b');
assert.equal(member.parameters?.[1].description, 'Param b description');
assert.equal(member.parameters?.[1].type?.text, 'boolean');
assert.equal(member.parameters?.[1].optional, true);
assert.equal(member.parameters?.[1].default, 'false');
assert.equal(member.parameters?.[1].rest, false);
assert.equal(member.parameters?.[2].name, 'c');
assert.equal(member.parameters?.[2].description, 'Param c description');
assert.equal(member.parameters?.[2].summary, undefined);
assert.equal(member.parameters?.[2].type?.text, 'number[]');
assert.equal(member.parameters?.[2].optional, false);
assert.equal(member.parameters?.[2].default, undefined);
assert.equal(member.parameters?.[2].rest, true);
assert.equal(member.return?.type?.text, 'string');
assert.equal(
member.return?.description,
'Static method 1 return description'
);
assert.equal(member.deprecated, 'Static method 1 deprecated');
});

test.run();

// Doing module JSDoc tests in-memory, to test a number of variations
Expand Down
21 changes: 21 additions & 0 deletions packages/labs/analyzer/test-files/js/jsdoc/element-a.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,27 @@ export class ElementA extends LitElement {
method2(a, b = false, ...c) {
return b ? a : c[0].toFixed();
}

/**
* @summary Static class field 1 summary
* @description Static class field 1 description
* @protected
* @type {number | string}
*/
static field1;

/**
* @summary Static method 1 summary
* @description Static method 1 description
* @param {string} a Param a description
* @param {boolean} b Param b description
* @param {number[]} c Param c description
* @returns {string} Static method 1 return description
* @deprecated Static method 1 deprecated
*/
static method1(a, b = false, ...c) {
return b ? a : c[0].toFixed();
}
}
customElements.define('element-a', ElementA);

Expand Down
20 changes: 20 additions & 0 deletions packages/labs/analyzer/test-files/ts/jsdoc/src/element-a.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,26 @@ export class ElementA extends LitElement {
method2(a: string, b = false, ...c: number[]) {
return b ? a : c[0].toFixed();
}

/**
* @summary Static class field 1 summary
* @description Static class field 1 description
* @protected
*/
static field1: number | string;

/**
* @summary Static method 1 summary
* @description Static method 1 description
* @param a Param a description
* @param b Param b description
* @param c Param c description
* @returns Static method 1 return description
* @deprecated Static method 1 deprecated
*/
static method1(a: string, b = false, ...c: number[]) {
return b ? a : c[0].toFixed();
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,12 @@
"summary": "My awesome element",
"superclass": {"name": "LitElement", "package": "lit"},
"members": [
{
"kind": "field",
"name": "foo",
"privacy": "public",
"type": {"text": "string | undefined"}
},
{
"kind": "field",
"name": "styles",
Expand All @@ -32,12 +38,6 @@
},
"default": "css`\n :host {\n display: block;\n background-color: var(--background-color);\n color: var(-foreground-color);\n }\n `"
},
{
"kind": "field",
"name": "foo",
"privacy": "public",
"type": {"text": "string | undefined"}
},
{
"kind": "method",
"name": "render",
Expand Down
2 changes: 2 additions & 0 deletions packages/labs/gen-manifest/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,9 @@ const convertClassDeclaration = (
// mixins: [], // TODO
members: ifNotEmpty([
...Array.from(declaration.fields).map(convertClassField),
...Array.from(declaration.staticFields).map(convertClassField),
...Array.from(declaration.methods).map(convertClassMethod),
...Array.from(declaration.staticMethods).map(convertClassMethod),
]),
// source: {href: 'TODO'}, // TODO
};
Expand Down