Skip to content

Commit

Permalink
feat: Component best practices- async flags check (#73)
Browse files Browse the repository at this point in the history
JIRA: CPOUI5FOUNDATION-792

---------

Co-authored-by: Merlin Beutlberger <[email protected]>
  • Loading branch information
d3xter666 and RandomByte authored May 29, 2024
1 parent 60d63f0 commit 1c58105
Show file tree
Hide file tree
Showing 61 changed files with 2,201 additions and 81 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,11 @@ The extracted and generated metadata is stored within the repository under the `
Regular updates to the metadata are necessary to ensure that the data is compatible with the corresponding UI5 type definitions.

```sh
npm run update-pseudo-modules-info -- $DOMAIN_NAME/com/sap/ui5/dist/sapui5-sdk-dist/1.120.12/sapui5-sdk-dist-1.120.12-api-jsons.zip 1.120.12
npm run update-pseudo-modules-info -- $DOMAIN_NAME/com/sap/ui5/dist/sapui5-sdk-dist/1.120.13/sapui5-sdk-dist-1.120.13-api-jsons.zip 1.120.13
```

```sh
npm run update-semantic-model-info -- $DOMAIN_NAME/com/sap/ui5/dist/sapui5-sdk-dist/1.120.12/sapui5-sdk-dist-1.120.12-api-jsons.zip 1.120.12
npm run update-semantic-model-info -- $DOMAIN_NAME/com/sap/ui5/dist/sapui5-sdk-dist/1.120.13/sapui5-sdk-dist-1.120.13-api-jsons.zip 1.120.13
```


Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"check-licenses": "licensee --errors-only",
"cleanup": "rimraf lib coverage",
"coverage": "nyc ava --node-arguments=\"--experimental-loader=@istanbuljs/esm-loader-hook\"",
"depcheck": "depcheck --ignores @commitlint/config-conventional,@istanbuljs/esm-loader-hook,@sapui5/types,@ui5/logger,ava,rimraf,sap,tsx,json-source-map,he,@types/he",
"depcheck": "depcheck --ignores @commitlint/config-conventional,@istanbuljs/esm-loader-hook,@sapui5/types,@ui5/logger,ava,rimraf,sap,tsx,json-source-map,he,@types/he,mycomp",
"hooks:pre-push": "npm run lint:commit",
"lint": "eslint .",
"lint:commit": "commitlint -e",
Expand Down
2 changes: 1 addition & 1 deletion resources/api-extract.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"framework": {
"name": "SAPUI5",
"version": "1.120.12"
"version": "1.120.13"
},
"defaultAggregations": {
"sap.ca.ui.CustomerControlListItem": "content",
Expand Down
5 changes: 5 additions & 0 deletions src/linter/LinterContext.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {AbstractAdapter, AbstractReader} from "@ui5/fs";
import {createReader} from "@ui5/fs/resourceFactory";
import {resolveLinks} from "../formatter/lib/resolveLinks.js";

export type FilePath = string; // Platform-dependent path
export type ResourcePath = string; // Always POSIX
Expand Down Expand Up @@ -156,6 +157,10 @@ export default class LinterContext {
}

addLintingMessage(resourcePath: ResourcePath, message: LintMessage) {
if (message.messageDetails) {
message.messageDetails = resolveLinks(message.messageDetails);
}

this.getLintingMessages(resourcePath).push(message);
}

Expand Down
2 changes: 1 addition & 1 deletion src/linter/linter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export async function lintFile({
}: LinterOptions): Promise<LintResult[]> {
const reader = createReader({
fsBasePath: rootDir,
virBasePath: "/",
virBasePath: namespace ? `/resources/${namespace}/` : "/",
});
let resolvedFilePaths;
if (pathsToLint?.length) {
Expand Down
22 changes: 21 additions & 1 deletion src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import ts, {Identifier} from "typescript";
import path from "node:path/posix";
import SourceFileReporter from "./SourceFileReporter.js";
import LinterContext, {ResourcePath, CoverageCategory, LintMessageSeverity} from "../LinterContext.js";
import analyzeComponentJson from "./asyncComponentFlags.js";

interface DeprecationInfo {
symbol: ts.Symbol;
Expand All @@ -16,11 +18,14 @@ export default class SourceFileLinter {
#boundVisitNode: (node: ts.Node) => void;
#reportCoverage: boolean;
#messageDetails: boolean;
#manifestContent: string | undefined;
#fileName: string;
#isComponent: boolean;

constructor(
context: LinterContext, resourcePath: ResourcePath, sourceFile: ts.SourceFile, sourceMap: string | undefined,
checker: ts.TypeChecker, reportCoverage: boolean | undefined = false,
messageDetails: boolean | undefined = false
messageDetails: boolean | undefined = false, manifestContent?: string | undefined
) {
this.#resourcePath = resourcePath;
this.#sourceFile = sourceFile;
Expand All @@ -30,6 +35,9 @@ export default class SourceFileLinter {
this.#boundVisitNode = this.visitNode.bind(this);
this.#reportCoverage = reportCoverage;
this.#messageDetails = messageDetails;
this.#manifestContent = manifestContent;
this.#fileName = path.basename(resourcePath);
this.#isComponent = this.#fileName === "Component.js" || this.#fileName === "Component.ts";
}

// eslint-disable-next-line @typescript-eslint/require-await
Expand Down Expand Up @@ -67,6 +75,15 @@ export default class SourceFileLinter {
node as (ts.PropertyAccessExpression | ts.ElementAccessExpression)); // Check for deprecation
} else if (node.kind === ts.SyntaxKind.ImportDeclaration) {
this.analyzeImportDeclaration(node as ts.ImportDeclaration); // Check for deprecation
} else if (node.kind === ts.SyntaxKind.ExpressionWithTypeArguments && this.#isComponent) {
analyzeComponentJson({
node: node as ts.ExpressionWithTypeArguments,
manifestContent: this.#manifestContent,
resourcePath: this.#resourcePath,
reporter: this.#reporter,
context: this.#context,
checker: this.#checker,
});
}

// Traverse the whole AST from top to bottom
Expand Down Expand Up @@ -169,6 +186,9 @@ export default class SourceFileLinter {
// returned by a class constructor.
// However, the OPA Matchers are a known exception where constructors do return a function.
return;
} else if (exprNode.kind === ts.SyntaxKind.SuperKeyword) {
// Ignore super calls
return;
}

if (!ts.isPropertyAccessExpression(exprNode) &&
Expand Down
12 changes: 10 additions & 2 deletions src/linter/ui5Types/TypeLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import SourceFileLinter from "./SourceFileLinter.js";
import {taskStart} from "../../util/perf.js";
import {getLogger} from "@ui5/logger";
import LinterContext, {LinterParameters} from "../LinterContext.js";
// import {Project} from "@ui5/project";
import path from "node:path/posix";
import {AbstractAdapter} from "@ui5/fs";
import {createAdapter, createResource} from "@ui5/fs/resourceFactory";

Expand Down Expand Up @@ -103,12 +103,20 @@ export default class TypeChecker {
if (!sourceMap) {
log.verbose(`Failed to get source map for ${sourceFile.fileName}`);
}
let manifestContent;
if (sourceFile.fileName.endsWith("/Component.js") || sourceFile.fileName.endsWith("/Component.ts")) {
const res = await this.#workspace.byPath(path.dirname(sourceFile.fileName) + "/manifest.json");
if (res) {
manifestContent = await res.getString();
}
}
const linterDone = taskStart("Type-check resource", sourceFile.fileName, true);
const linter = new SourceFileLinter(
this.#context,
sourceFile.fileName, sourceFile,
sourceMap,
checker, reportCoverage, messageDetails
checker, reportCoverage, messageDetails,
manifestContent
);
await linter.lint();
linterDone();
Expand Down
Loading

0 comments on commit 1c58105

Please sign in to comment.