Skip to content

Commit 2a50fdc

Browse files
marklundinCopilot
marklundin
and
Copilot
authored
Enhance parsing error handling and attribute validation (#53)
* Enhance parsing error handling and attribute validation - Introduced a new `ParsingError` class to encapsulate error details, including a fix suggestion. - Updated `JSDocParser` to collect parsing errors for scripts missing a `scriptName` property. - Enhanced `AttributeParser` to provide more informative error messages and suggested fixes for invalid tags. - Refactored utility functions to improve tag retrieval and validation. - Updated tests to ensure accurate error reporting for missing script names and invalid attribute types. * Update src/parsers/attribute-parser.js Co-authored-by: Copilot <[email protected]> * Refactor JSDocParser and enhance error messages in ScriptParser - Removed the localResults object in JSDocParser and directly returned results for better clarity. - Improved error messages in ScriptParser to specify valid attribute types, enhancing user guidance for invalid types. * Refactor JSDoc type definition in ScriptParser - Updated the JSDoc type annotation for SUPPORTED_BLOCK_TAGS to use a more descriptive TagDefinition type, improving code clarity and maintainability. * Enhance JSDoc type definition in AttributeParser - Updated the JSDoc type annotation for the tags parameter in the AttributeParser constructor to use a more descriptive TagDefinition type, improving code clarity and maintainability. --------- Co-authored-by: Copilot <[email protected]>
1 parent 9d90553 commit 2a50fdc

35 files changed

+301
-92
lines changed

‎src/index.js‎

Lines changed: 59 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,24 @@
11
import { createSystem, createDefaultMapFromNodeModules, createVirtualTypeScriptEnvironment } from '@typescript/vfs';
22
import * as ts from 'typescript';
33

4+
import { ParsingError } from './parsers/parsing-error.js';
45
import { ScriptParser } from './parsers/script-parser.js';
5-
import { isInterface, isStaticMember } from './utils/attribute-utils.js';
6-
import { createDefaultMapFromCDN, flatMapAnyNodes, getExportedNodes, getType, inheritsFrom, isAliasedClassDeclaration } from './utils/ts-utils.js';
6+
import { isStaticMember } from './utils/attribute-utils.js';
7+
import { createDefaultMapFromCDN, getExportedNodes, getType, inheritsFrom, isAliasedClassDeclaration } from './utils/ts-utils.js';
78

89
const toLowerCamelCase = str => str[0].toLowerCase() + str.substring(1);
910

11+
const SUPPORTED_ATTRIBUTE_TYPES = new Set([
12+
'Curve',
13+
'Vec2',
14+
'Vec3',
15+
'Vec4',
16+
'Color',
17+
'number',
18+
'string',
19+
'boolean'
20+
]);
21+
1022
const COMPILER_OPTIONS = {
1123
noLib: true,
1224
strict: false,
@@ -110,9 +122,10 @@ export class JSDocParser {
110122
/**
111123
* Returns all the valid ESM Scripts within a file
112124
* @param {string} fileName - The file name in the program to check
125+
* @param {ParsingError[]} errors - An array to store any parsing errors
113126
* @returns {Map<string, import('typescript').Node>} - A map of valid ESM Script <names, nodes> within the file
114127
*/
115-
getAllEsmScripts(fileName) {
128+
getAllEsmScripts(fileName, errors = []) {
116129

117130
const typeChecker = this.program.getTypeChecker();
118131

@@ -163,8 +176,27 @@ export class JSDocParser {
163176
if (scriptNameMember) {
164177
finalName = scriptNameMember.initializer.text;
165178
} else {
166-
// Otherwise, convert the class name to lower camel case
167179
finalName = toLowerCamelCase(name);
180+
const insertOffset = node.members.pos;
181+
const insertPos = node.getSourceFile().getLineAndCharacterOfPosition(insertOffset);
182+
183+
errors.push(
184+
new ParsingError(
185+
node,
186+
'Missing Script Name',
187+
'Scripts should have a static scriptName member that identifies the script.',
188+
{
189+
title: 'Add scriptName',
190+
text: `\n static scriptName = '${finalName}';\n`,
191+
range: {
192+
startLineNumber: insertPos.line + 1,
193+
startColumn: insertPos.character + 1,
194+
endLineNumber: insertPos.line + 1,
195+
endColumn: insertPos.character + 1
196+
}
197+
}
198+
)
199+
);
168200
}
169201

170202
esmScripts.set(finalName, node);
@@ -189,7 +221,7 @@ export class JSDocParser {
189221
}
190222

191223
// Extract all exported nodes
192-
const nodes = this.getAllEsmScripts(fileName);
224+
const nodes = this.getAllEsmScripts(fileName, errors);
193225

194226
// Extract attributes from each script
195227
nodes.forEach((node, name) => {
@@ -214,41 +246,31 @@ export class JSDocParser {
214246
}
215247

216248
const typeChecker = this.program.getTypeChecker();
249+
const esmScripts = this.getAllEsmScripts(fileName, errors);
250+
const [attributes] = this.parseAttributes(fileName);
217251

218-
// Find the Script class in the PlayCanvas namespace
219-
const pcTypes = this.program.getSourceFile('/playcanvas.d.ts');
220-
const esmScriptClass = pcTypes.statements.find(node => node.kind === ts.SyntaxKind.ClassDeclaration && node.name.text === 'Script')?.symbol;
221-
222-
// Parse the source file and pc types
223-
const sourceFile = this.program.getSourceFile(fileName);
224-
225-
if (!sourceFile) {
226-
throw new Error(`Source file ${fileName} not found`);
227-
}
228-
229-
const nodes = flatMapAnyNodes(sourceFile, (node) => {
230-
if (!ts.isClassDeclaration(node)) {
231-
return false;
232-
}
233-
234-
return inheritsFrom(node, typeChecker, esmScriptClass) || isInterface(node);
235-
});
236-
237-
for (const node of nodes) {
238-
const name = toLowerCamelCase(node.name.text);
252+
const scripts = [];
253+
for (const [scriptName, node] of esmScripts) {
239254
const members = [];
240-
241255
for (const member of node.members) {
242-
if (member.kind !== ts.SyntaxKind.PropertyDeclaration || isStaticMember(member)) {
243-
continue;
256+
if (member.kind === ts.SyntaxKind.PropertyDeclaration && !isStaticMember(member)) {
257+
members.push(member);
244258
}
259+
}
260+
scripts.push({ scriptName, members });
261+
errors.push(...(attributes[scriptName]?.errors ?? []));
262+
}
245263

246-
// Extract JSDoc tags
247-
const tags = member.jsDoc ? member.jsDoc.map(jsdoc => jsdoc.tags?.map(tag => tag.tagName.getText()) ?? []).flat() : [];
248-
264+
for (const { scriptName, members } of scripts) {
265+
const localMembers = [];
266+
for (const member of members) {
249267
const name = member.name.getText();
268+
const attribute = attributes[scriptName].attributes[name];
269+
const isAttribute = !!attribute;
270+
250271
const type = getType(member, typeChecker);
251-
if (!type) {
272+
273+
if (!SUPPORTED_ATTRIBUTE_TYPES.has(type.name)) {
252274
continue;
253275
}
254276

@@ -257,19 +279,17 @@ export class JSDocParser {
257279
const jsdocNode = member.jsDoc && member.jsDoc[member.jsDoc.length - 1];
258280
const jsdocPos = jsdocNode ? ts.getLineAndCharacterOfPosition(member.getSourceFile(), jsdocNode.getStart()) : null;
259281

260-
const data = {
282+
localMembers.push({
261283
name,
284+
isAttribute,
262285
type: type.name + (type.array ? '[]' : ''),
263-
isAttribute: tags.includes('attribute'),
264286
start: jsdocNode ? { line: jsdocPos.line + 1, column: jsdocPos.character + 1 } :
265287
{ line: namePos.line + 1, column: namePos.character + 1 },
266288
end: { line: namePos.line + 1, column: namePos.character + name.length + 1 }
267-
};
268-
269-
members.push(data);
289+
});
270290
}
271291

272-
results[name] = members;
292+
results[scriptName] = localMembers;
273293
}
274294

275295
return [results, errors];

‎src/parsers/attribute-parser.js‎

Lines changed: 45 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,21 @@ import { hasTag } from '../utils/attribute-utils.js';
55
import { parseTag, validateTag } from '../utils/tag-utils.js';
66
import { getLiteralValue, getType } from '../utils/ts-utils.js';
77

8+
/**
9+
* @typedef {Object} TagDefinition
10+
* @property {string} type - The type of the tag
11+
* @property {() => string} supportMessage - A function to generate a support message for the tag
12+
* @property {string} fix - A function to fix the tag
13+
*/
14+
815
/**
916
* A class to parse JSDoc comments and extract attribute metadata.
1017
*/
1118
export class AttributeParser {
1219
/**
1320
* Creates a new instance of the ScriptParser class.
1421
*
15-
* @param {Map<string, string>} tags - An set of tag definitions and their validators to add to the parser
22+
* @param {Map<string, TagDefinition>} tags - An set of tag definitions and their validators to add to the parser
1623
* @param {object} env - The TypeScript environment to use
1724
* @param {Map<string, Function>} typeSerializerMap - A map of custom serializers
1825
*/
@@ -52,24 +59,54 @@ export class AttributeParser {
5259
const jsDocTags = node.jsDoc?.[0]?.tags ?? [];
5360
jsDocTags.forEach((tag) => {
5461
// Only use the first line of the comment
62+
const tagName = tag.tagName.text;
5563
let commentText = (tag.comment?.split('\n')[0] || '').trim();
5664

5765
// Check if the tag is a supported tag
58-
if (this.tagTypeAnnotations.has(tag.tagName.text)) {
66+
if (this.tagTypeAnnotations.has(tagName)) {
67+
const { type, supportMessage, fix } = this.tagTypeAnnotations.get(tagName);
5968
try {
6069
const value = parseTag(commentText);
61-
const tagTypeAnnotation = this.tagTypeAnnotations.get(tag.tagName.text);
6270

6371
// Tags like @resource with string values do not need quotations, so we need to manually add them
6472
if (typeof value === 'string') commentText = `"${commentText}"`;
6573

66-
validateTag(commentText, tagTypeAnnotation, this.env);
67-
attribute[tag.tagName.text] = value;
74+
validateTag(commentText, type, this.env);
75+
attribute[tagName] = value;
6876

6977
} catch (error) {
70-
const file = node.getSourceFile();
71-
const { line, character } = file.getLineAndCharacterOfPosition(tag.getStart());
72-
const parseError = new ParsingError(node, `Invalid Tag '@${tag.tagName.text}'`, `Error (${line}, ${character}): Parsing Tag '@${tag.tagName.text} ${commentText}' - ${error.message}`);
78+
79+
// generate a fix if one is provided
80+
let startPos = tag.getStart();
81+
let endPos = tag.getEnd();
82+
83+
const fullText = tag.getText();
84+
const commentText = tag.comment?.split('\n')[0]?.trim() || '';
85+
86+
// Find the start of the comment content within the full text
87+
const commentStart = fullText.indexOf(commentText);
88+
if (commentStart !== -1) {
89+
startPos = tag.getStart() + commentStart;
90+
endPos = startPos + commentText.length;
91+
}
92+
93+
const sourceFile = tag.getSourceFile();
94+
const startLineChar = sourceFile.getLineAndCharacterOfPosition(startPos);
95+
const endLineChar = sourceFile.getLineAndCharacterOfPosition(endPos);
96+
97+
const edit = fix ? {
98+
text: fix,
99+
title: `Fix @${tagName} tag`,
100+
range: {
101+
startLineNumber: startLineChar.line + 1,
102+
startColumn: startLineChar.character + 1,
103+
endLineNumber: endLineChar.line + 1,
104+
endColumn: endLineChar.character + 1
105+
}
106+
} : null;
107+
108+
const errorMessage = supportMessage?.() ?? 'The tag is invalid.';
109+
const parseError = new ParsingError(tag, `Invalid Tag '@${tagName}'`, errorMessage, edit);
73110
errors.push(parseError);
74111
}
75112
}

‎src/parsers/parsing-error.js‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,23 @@
1+
/**
2+
* @typedef {Object} Fix
3+
* @property {string} text - The text to insert at the fix location
4+
* @property {string} title - The title of the fix
5+
* @property {{ startLineNumber: number, startColumn: number, endLineNumber: number, endColumn: number }} range - The range of the fix
6+
*/
7+
8+
/**
9+
* A class representing a parsing error.
10+
* @param {import('typescript').Node} node - The AST node which caused the error
11+
* @param {string} type - The type of the error
12+
* @param {string} message - The description of the error
13+
* @param {Fix} [fix] - The fix for the error
14+
*/
115
export class ParsingError {
2-
constructor(node, type, message) {
16+
constructor(node, type, message, fix) {
317
this.node = node; // AST node which caused the error
418
this.type = type; // Type of the error
519
this.message = message; // Description of the error
20+
this.fix = fix; // Fix for the error
621
}
722

823
toString() {

0 commit comments

Comments
 (0)