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

feat: Component best practices- async flags check #73

Merged
merged 90 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
90 commits
Select commit Hold shift + click to select a range
e2bab81
feat: Analyze async flags
d3xter666 Apr 15, 2024
46b4622
feat: Implement manifest checks
d3xter666 Apr 15, 2024
6e32f58
feat: Extend linting further
d3xter666 Apr 16, 2024
cda4880
fix: JSON "parse" improvement
d3xter666 Apr 16, 2024
c980c07
feat: Handle variations for error messaging
d3xter666 Apr 16, 2024
7d5711d
feat: Checks for inline manifest
d3xter666 Apr 18, 2024
4e8d607
fix: Tests
d3xter666 Apr 18, 2024
3280f54
fix: Merge conflicts
d3xter666 Apr 18, 2024
a3899a0
fix: Reset Component.js
d3xter666 Apr 18, 2024
2a8c08e
fix: Tests
d3xter666 Apr 18, 2024
a02fbad
refactor: Restructure rules tests to use a common execution
d3xter666 Apr 19, 2024
baaeb79
refactor: Linter helper
d3xter666 Apr 19, 2024
9358dd4
feat: Add tests for Best practices
d3xter666 Apr 19, 2024
7524834
fix: Tests
d3xter666 Apr 19, 2024
daeb315
refactor: Automatically resolve BestPractices subdirs
d3xter666 Apr 19, 2024
badf560
feat: Add more tests for Best Practices
d3xter666 Apr 19, 2024
1f998ba
fix: Detection of unset values
d3xter666 Apr 19, 2024
acd3472
refactor: Improve messaging
d3xter666 Apr 19, 2024
89b1261
fix: Eslint
d3xter666 Apr 19, 2024
1611ad9
fix: Update snapshots
d3xter666 Apr 19, 2024
5dc3f12
refactor: Interfaces are part of metadata
d3xter666 Apr 22, 2024
894029c
refactor: Look into hierarchy
d3xter666 Apr 24, 2024
b88f623
fix: Extract and check against real module dependency var
d3xter666 Apr 24, 2024
aedea65
refactor: Allow sourcefile tests for a whole dir
d3xter666 Apr 25, 2024
079444b
test: Add BestPractices samples
d3xter666 Apr 25, 2024
8bb2b38
fix: Allow filtering for BestPractices tests
d3xter666 Apr 25, 2024
c78eaca
fix: Analyze only Component.js files
d3xter666 Apr 25, 2024
b95900c
test: Add negative test case
d3xter666 Apr 25, 2024
ddf092d
fix: Test Component's namespace
d3xter666 Apr 25, 2024
e94be3f
fix: Tests
d3xter666 Apr 25, 2024
455267a
fix: Eslint
d3xter666 Apr 25, 2024
6199213
test: Add positive case
d3xter666 Apr 25, 2024
aa6de7c
fix: Update snapshots
d3xter666 Apr 25, 2024
d64b365
fix: Windows paths for tests
d3xter666 Apr 25, 2024
c2f2ece
fix: Paths on windows
d3xter666 Apr 25, 2024
292334a
refactor: Cleanup obsolete code
d3xter666 Apr 25, 2024
5a9aa70
refactor: Cleanup code
d3xter666 Apr 26, 2024
1b1a277
refactor: Always try to process the manifest
d3xter666 Apr 26, 2024
c410868
refactor: Detect sap/ui/core/UIComponent dependency
d3xter666 Apr 26, 2024
95d29c4
refactor: Lint Message
d3xter666 Apr 26, 2024
52dd0d5
fix: Update messages & tests
d3xter666 Apr 26, 2024
5338f9c
feat: Detect manifest.json section in Component.js
d3xter666 Apr 26, 2024
05442d5
fix: Detect missing manifest declaration in Component.js
d3xter666 Apr 26, 2024
1091024
fix: Linting test filenames
d3xter666 Apr 26, 2024
546cb41
fix: Eslint
d3xter666 Apr 26, 2024
29f8cb7
fix: Paths for windows
d3xter666 Apr 26, 2024
1c6ee18
feat: Handle ES6 interface implementation
d3xter666 May 7, 2024
4d9e33a
test: Checks for inheritance in ES6 components
d3xter666 May 7, 2024
e2238e9
refactor: Renames & docs
d3xter666 May 8, 2024
013122f
fix: Eslint issues
d3xter666 May 8, 2024
af1d2f9
fix: Handle quoted properties
d3xter666 May 15, 2024
29fa1db
test: Add case for missing view & routing
d3xter666 May 15, 2024
a7a307e
refactor: Migrate checks to enum
d3xter666 May 15, 2024
79cdbe7
fix: Reset result template
d3xter666 May 15, 2024
89f11f8
refactor: Remove redundant comments
d3xter666 May 15, 2024
bd2f0da
refactor: Rename BestPractices
d3xter666 May 15, 2024
9679220
refactor: Exit early
d3xter666 May 15, 2024
0cd5a7f
refactor: Remove redundant checks
d3xter666 May 15, 2024
a8e62d2
refactor: Resolve SourceFile
d3xter666 May 15, 2024
da987fd
refactor: Cleanup redundant code
d3xter666 May 15, 2024
cacf900
refactor: Improve messaging
d3xter666 May 15, 2024
4d8a01c
fix: Eslint
d3xter666 May 15, 2024
c30a8c9
fix: Rename bestPractices.ts
d3xter666 May 15, 2024
adae2ac
refactor: Extract virBasePath from filePaths
d3xter666 May 16, 2024
b3c1bb5
feat: Provide links to manifest.json when needed
d3xter666 May 17, 2024
168fd14
fix: Link resolver for LinterContext
d3xter666 May 17, 2024
697869c
test: Update snapshots with the new changes
d3xter666 May 17, 2024
f840097
fix: Eslint
d3xter666 May 17, 2024
c9319d9
fix: LintContext fix links resolve
d3xter666 May 17, 2024
49d4e33
refactor: Improve messaging
d3xter666 May 17, 2024
34f6698
refactor: Improve runtime performance by checking Component.js
d3xter666 May 17, 2024
1bb546d
refactor: Cleanup
d3xter666 May 17, 2024
e670ecc
test: Add case for obsolete async flag
d3xter666 May 17, 2024
ca2115e
test: Update snapshots
d3xter666 May 17, 2024
417a94b
fix: Correct logic for reporting
d3xter666 May 17, 2024
604ef22
test: Add test cases for single async flags
d3xter666 May 17, 2024
6d63c56
fix: Report missing manifest definition only when there is a manifest…
d3xter666 May 20, 2024
77e5369
deps: Rebuild api-json deps from UI5 1.120.13
d3xter666 May 27, 2024
0b01e1b
refactor(tests): Rename BestPractices to AsyncComponentFlags, use dir…
RandomByte May 23, 2024
84dfaf2
refactor: Rename bestPractices module to asyncComponentFlags, also ch…
RandomByte May 23, 2024
d2eabaa
test(AsyncComponentFlags): Add more test cases
RandomByte May 23, 2024
e7a8955
refactor(asyncComponentFlags): Remove usage of internal API, improve …
RandomByte May 24, 2024
608dd67
refactor(asyncComponentFlags): Rename types, use boolean for async in…
RandomByte May 24, 2024
a718397
refactor(asyncComponentFlags): Simplify async flag merge logic
RandomByte May 24, 2024
992dee4
test(AsyncComponentFlags): Move parent components into subdirs
RandomByte May 27, 2024
f4be3cb
fix: Skip depcheck for fake modules in testing
d3xter666 May 28, 2024
85fda50
refactor: Improve file path handling
RandomByte May 28, 2024
7790b4a
refactor: Update message texts
RandomByte May 28, 2024
f136e64
refactor: Fix message text
RandomByte May 28, 2024
8e42467
refactor: Revert "refactor: Extract virBasePath from filePaths"
RandomByte May 28, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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",

Check warning on line 36 in package.json

View check run for this annotation

In Solidarity / Inclusive Language

Match Found

You might want to consider an alternative to the term "he". Possibilities might include: "they", "them", "their". (This check uses pattern-matching and may therefore be incorrect. To improve the patterns or to make other suggestions, please open an issue or pull-request at https://github.com/ietf/terminology/issues.)
Raw output
/\b(he|him|his|she|her|hers)\b/gi

Check warning on line 36 in package.json

View check run for this annotation

In Solidarity / Inclusive Language

Match Found

You might want to consider an alternative to the term "he". Possibilities might include: "they", "them", "their". (This check uses pattern-matching and may therefore be incorrect. To improve the patterns or to make other suggestions, please open an issue or pull-request at https://github.com/ietf/terminology/issues.)
Raw output
/\b(he|him|his|she|her|hers)\b/gi
"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}/` : "/",
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
});
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