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: Detect usage of non exported values by library.js #468

Merged
merged 25 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4de7e91
feat: Add new message type
d3xter666 Jan 9, 2025
595e0d3
feat: Detect exported values by library
d3xter666 Jan 9, 2025
5f1e58a
refactor: Use getter for the name
d3xter666 Jan 9, 2025
57b7956
test: Add test cases
d3xter666 Jan 9, 2025
c51cb02
fix: Breaking check
d3xter666 Jan 9, 2025
5b5f3db
test: Add more test cases
d3xter666 Jan 9, 2025
c11d7a8
refactor: Add support for PropertyAccessExpression-s
d3xter666 Jan 9, 2025
3cb4e5e
test: Update snapshots
d3xter666 Jan 9, 2025
68ee498
test: Fix test cases
d3xter666 Jan 9, 2025
0c2b254
refactor: Improve messaging
d3xter666 Jan 9, 2025
5207d26
fix: Allow ElementAccessExpressions
d3xter666 Jan 10, 2025
ff8910e
test: Add more test cases
d3xter666 Jan 10, 2025
3a5a668
docs: Update comments
d3xter666 Jan 10, 2025
8bb9d55
fix: Merge conflicts
d3xter666 Jan 10, 2025
82a2920
refactor: Remove redundant check
d3xter666 Jan 10, 2025
9a39a75
test: Remove redundant tests
d3xter666 Jan 10, 2025
ccf7380
fix: Properly detect non exported elements
d3xter666 Jan 10, 2025
46c7683
test: Update snapshots with the correct findings
d3xter666 Jan 10, 2025
b5dccb3
refactor: Exported values messaging
d3xter666 Jan 10, 2025
d6e3f0f
test: Update snapshots w/ the new messages
d3xter666 Jan 10, 2025
2cbb8f4
refactor: Restore the non exported negative examples
d3xter666 Jan 10, 2025
ef38758
refactor: Update messages
d3xter666 Jan 10, 2025
c6bde70
fix: Resolve nested namespaces
d3xter666 Jan 13, 2025
734d1fb
test: Update snapshots
d3xter666 Jan 13, 2025
3d050be
refactor: Move sample to the negative test
d3xter666 Jan 13, 2025
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
11 changes: 11 additions & 0 deletions src/linter/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const RULES = {
"no-deprecated-library": "no-deprecated-library",
"no-deprecated-theme": "no-deprecated-theme",
"no-globals": "no-globals",
"no-implicit-globals": "no-implicit-globals",
"no-pseudo-modules": "no-pseudo-modules",
"parsing-error": "parsing-error",
"ui5-class-declaration": "ui5-class-declaration",
Expand Down Expand Up @@ -53,6 +54,7 @@ export enum MESSAGE {
NO_DEPRECATED_RENDERER,
NO_DIRECT_DATATYPE_ACCESS,
NO_DIRECT_ENUM_ACCESS,
NO_EXPORTED_VALUES_BY_LIB,
NO_GLOBALS,
NO_ICON_POOL_RENDERER,
NOT_STATIC_CONTROL_RENDERER,
Expand Down Expand Up @@ -570,4 +572,13 @@ export const MESSAGE_INFO = {
details: ({messageDetails}: {messageDetails: string}) => messageDetails,
},

[MESSAGE.NO_EXPORTED_VALUES_BY_LIB]: {
severity: LintMessageSeverity.Error,
ruleId: RULES["no-implicit-globals"],

message: ({module}: {module: string}) => `Access of a module (${module}) not exported by a library`,
details: ({namespace}: {namespace: string}) =>
`Please import the module itself (${namespace}) because it is no longer exported by the library`,
},

} as const;
56 changes: 56 additions & 0 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ export default class SourceFileLinter {
node as (ts.PropertyAccessExpression | ts.ElementAccessExpression)); // Check for global
this.analyzePropertyAccessExpressionForDeprecation(
node as (ts.PropertyAccessExpression | ts.ElementAccessExpression)); // Check for deprecation
this.analyzeExportedValuesByLib(node as (ts.PropertyAccessExpression | ts.ElementAccessExpression));
} else if (node.kind === ts.SyntaxKind.ObjectBindingPattern &&
node.parent?.kind === ts.SyntaxKind.VariableDeclaration) {
// e.g. `const { Button } = sap.m;`
Expand Down Expand Up @@ -1185,8 +1186,63 @@ export default class SourceFileLinter {
].includes(nodeType);
}

analyzeExportedValuesByLib(node: ts.PropertyAccessExpression | ts.ElementAccessExpression) {
if (!ts.isElementAccessExpression(node) &&
node.name?.kind !== ts.SyntaxKind.Identifier) {
return;
}

const extractVarName = (node: ts.PropertyAccessExpression | ts.ElementAccessExpression) => {
const nodeName = ts.isPropertyAccessExpression(node) ?
node.name.getText() :
node.argumentExpression.getText();

return nodeName.replaceAll("\"", "");
};

let exprNode = node.expression;
const namespace: string[] = [];
while (ts.isPropertyAccessExpression(exprNode) ||
ts.isElementAccessExpression(exprNode)) {
namespace.unshift(extractVarName(exprNode));
exprNode = exprNode.expression;
}
const exprType = this.checker.getTypeAtLocation(exprNode);
const potentialLibImport = exprType.symbol?.getName().replaceAll("\"", "") ?? "";

// Checks if the left hand side is a library import.
// It's sufficient just to check for "/library" at the end of the string by convention
if (!potentialLibImport.endsWith("/library")) {
return;
}

const moduleName = [
potentialLibImport.replace("/library", ""),
...namespace,
extractVarName(node),
].join("/");

// Check if the module is registered within ambient modules
const ambientModules = this.checker.getAmbientModules();
const isRegisteredAsUi5Module = ambientModules.some((module) =>
module.name === `"${moduleName}"`);

if (isRegisteredAsUi5Module) {
this.#reporter.addMessage(MESSAGE.NO_EXPORTED_VALUES_BY_LIB, {
module: [
exprNode?.getText(),
...namespace,
extractVarName(node),
].join("/"),
namespace: moduleName,
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
},
(ts.isPropertyAccessExpression(node) ? node.name : node.argumentExpression));
}
}

analyzePropertyAccessExpression(node: ts.AccessExpression | ts.CallExpression) {
const exprNode = node.expression;

if (ts.isIdentifier(exprNode)) {
// The expression being an identifier indicates that this is the first access
// in a possible chain. E.g. the "sap" in "sap.ui.getCore()"
Expand Down
12 changes: 12 additions & 0 deletions test/fixtures/linter/rules/NoGlobals/NoExportedLibValues.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
sap.ui.define(["sap/ui/unified/library", "sap/ui/core/library"],
function (unifiedLibrary, coreLibrary) {
"use strict";

d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
var CalendarDayType = unifiedLibrary.CalendarDayType,
DateRange = unifiedLibrary["DateRange"],
DateTypeRange = unifiedLibrary.DateTypeRange,
DOMAttribute = coreLibrary.tmpl.DOMAttribute,
DOMAttribute2 = coreLibrary["tmpl"].DOMAttribute,
DOMAttribute3 = coreLibrary["tmpl"]["DOMAttribute"],
DOMAttribute4 = coreLibrary.tmpl["DOMAttribute"];
});
68 changes: 66 additions & 2 deletions test/lib/linter/rules/snapshots/Directives.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 9,
errorCount: 13,
fatalErrorCount: 0,
filePath: 'Directives.js',
messages: [
Expand Down Expand Up @@ -47,6 +47,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 10,
line: 65,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 2,
line: 68,
Expand All @@ -55,6 +63,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 10,
line: 68,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 2,
line: 72,
Expand All @@ -63,6 +79,22 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 10,
line: 72,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 12,
line: 93,
message: 'Access of a module (mobileLib/InputType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/InputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 6,
line: 108,
Expand Down Expand Up @@ -99,7 +131,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 9,
errorCount: 13,
fatalErrorCount: 0,
filePath: 'Directives.ts',
messages: [
Expand Down Expand Up @@ -135,6 +167,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 9,
line: 73,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 1,
line: 76,
Expand All @@ -143,6 +183,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 9,
line: 76,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 1,
line: 80,
Expand All @@ -151,6 +199,22 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 9,
line: 80,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 11,
line: 101,
message: 'Access of a module (mobileLib/InputType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/InputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 5,
line: 115,
Expand Down
Binary file modified test/lib/linter/rules/snapshots/Directives.ts.snap
Binary file not shown.
67 changes: 62 additions & 5 deletions test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 18,
errorCount: 20,
fatalErrorCount: 0,
filePath: 'Control_ManagedObject.js',
messages: [
Expand All @@ -600,6 +600,22 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 34,
line: 3,
message: 'Access of a module (library/DateTimeInputType) not exported by a library',
d3xter666 marked this conversation as resolved.
Show resolved Hide resolved
messageDetails: 'Please import the module itself (sap/m/DateTimeInputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 26,
line: 4,
message: 'Access of a module (library/FrameType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/FrameType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 17,
line: 8,
Expand Down Expand Up @@ -804,7 +820,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 18,
errorCount: 20,
fatalErrorCount: 0,
filePath: 'ManagedObject.js',
messages: [
Expand All @@ -816,6 +832,22 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 34,
line: 3,
message: 'Access of a module (library/DateTimeInputType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/DateTimeInputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 26,
line: 4,
message: 'Access of a module (library/FrameType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/FrameType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 17,
line: 8,
Expand Down Expand Up @@ -988,7 +1020,7 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 25,
errorCount: 27,
fatalErrorCount: 0,
filePath: 'NoDeprecatedApi.js',
messages: [
Expand Down Expand Up @@ -1128,6 +1160,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 10,
line: 37,
message: 'Access of a module (coreLib/MessageType) not exported by a library',
messageDetails: 'Please import the module itself (sap/ui/core/MessageType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 17,
line: 39,
Expand Down Expand Up @@ -1168,6 +1208,14 @@ Generated by [AVA](https://avajs.dev).
ruleId: 'no-deprecated-api',
severity: 2,
},
{
column: 12,
line: 47,
message: 'Access of a module (mobileLib/InputType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/InputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
{
column: 20,
line: 50,
Expand Down Expand Up @@ -1380,10 +1428,19 @@ Generated by [AVA](https://avajs.dev).
[
{
coverageInfo: [],
errorCount: 0,
errorCount: 1,
fatalErrorCount: 0,
filePath: 'NoDeprecatedApi_negative.js',
messages: [],
messages: [
{
column: 12,
line: 16,
message: 'Access of a module (mobileLib/InputType) not exported by a library',
messageDetails: 'Please import the module itself (sap/m/InputType) because it is no longer exported by the library',
ruleId: 'no-implicit-globals',
severity: 2,
},
],
warningCount: 0,
},
]
Expand Down
Binary file modified test/lib/linter/rules/snapshots/NoDeprecatedApi.ts.snap
Binary file not shown.
Loading
Loading