-
Notifications
You must be signed in to change notification settings - Fork 7
feat: Offer autofix for deprecated Core APIs #639
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
base: autofix-deprecated-core-base
Are you sure you want to change the base?
Changes from all commits
801b70d
99bbdc5
cddf911
928a16a
8b979dc
c808d0b
cede3d2
7b07648
643625c
28b4352
54f09ae
e52df10
4f8ae49
d03542b
45a67e7
eb046be
5429396
8e25bbd
38d1895
1022646
cbc6e6a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -314,11 +314,110 @@ function patchMessageFixHints(fixHints?: FixHints, apiName?: string) { | |
`$moduleIdentifier.${fnName}(${cleanRedundantArguments(fixHints.exportCodeToBeUsed.args)})`; | ||
} | ||
} | ||
} else if (apiName === "applyTheme" && fixHints?.moduleName === "sap/ui/core/Theming") { | ||
if ((fixHints?.exportCodeToBeUsed?.args?.length ?? 0) > 1 && | ||
fixHints?.exportCodeToBeUsed?.args?.[1]?.value !== "undefined") { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else if (["attachIntervalTimer", "detachIntervalTimer"].includes(apiName ?? "")) { | ||
if ((fixHints?.exportCodeToBeUsed?.args?.length ?? 0) > 1) { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else if (apiName === "loadLibrary") { | ||
if (fixHints?.exportCodeToBeUsed?.args?.[1]?.kind === SyntaxKind.ObjectLiteralExpression) { | ||
const libOptionsExpression = | ||
extractKeyValuePairs(fixHints.exportCodeToBeUsed.args[1].value) as {async: boolean; url?: string}; | ||
if (libOptionsExpression.async === true) { | ||
const newArg = { | ||
name: fixHints.exportCodeToBeUsed.args[0].value.replace(/^['"]+|['"]+$/g, ""), | ||
url: libOptionsExpression.url, | ||
}; | ||
fixHints.exportCodeToBeUsed.args[0].value = JSON.stringify(newArg); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here the comments in the code get lost. Also, the used quotes in the code will be always replaced by double-quotes. |
||
fixHints.exportCodeToBeUsed.args[0].kind = SyntaxKind.ObjectLiteralExpression; | ||
} else { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else if (fixHints?.exportCodeToBeUsed?.args?.[1]?.kind === SyntaxKind.TrueKeyword) { | ||
const newArg = { | ||
name: fixHints.exportCodeToBeUsed.args[0].value.replace(/^['"]+|['"]+$/g, ""), | ||
}; | ||
fixHints.exportCodeToBeUsed.args[0].value = JSON.stringify(newArg); | ||
fixHints.exportCodeToBeUsed.args[0].kind = SyntaxKind.ObjectLiteralExpression; | ||
} else { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else if (apiName === "createComponent") { | ||
if (fixHints?.exportCodeToBeUsed?.args?.[0]?.kind === SyntaxKind.ObjectLiteralExpression) { | ||
const componentOptionsExpression = | ||
extractKeyValuePairs(fixHints.exportCodeToBeUsed.args[0].value); | ||
if (componentOptionsExpression.async !== true) { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else { | ||
fixHints = undefined; // We cannot handle this case | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} else if (apiName === "getLibraryResourceBundle") { | ||
// Handling fallback in the legacy API | ||
if (!fixHints?.exportCodeToBeUsed?.args?.[0] || | ||
fixHints?.exportCodeToBeUsed?.args?.[0]?.value === "undefined") { | ||
fixHints.exportCodeToBeUsed.args ??= []; | ||
fixHints.exportCodeToBeUsed.args[0] = { | ||
value: "\"sap.ui.core\"", kind: SyntaxKind.StringLiteral, ui5Type: "library", | ||
}; | ||
} | ||
|
||
fixHints.exportCodeToBeUsed.name = `$moduleIdentifier` + | ||
`.getResourceBundleFor(${cleanRedundantArguments(fixHints.exportCodeToBeUsed.args ?? [])})`; | ||
|
||
// If any of the arguments is a boolean with value true, the return value is a promise | ||
// and is not compatible with the new API | ||
if ([fixHints?.exportCodeToBeUsed?.args?.[0]?.kind, | ||
fixHints?.exportCodeToBeUsed?.args?.[1]?.kind, | ||
fixHints?.exportCodeToBeUsed?.args?.[2]?.kind].includes(SyntaxKind.TrueKeyword) || | ||
fixHints?.exportCodeToBeUsed?.args?.[0]?.ui5Type !== "library") { | ||
fixHints = undefined; // The new API is async | ||
log.verbose(`Autofix skipped for ${apiName}. Transpilation is too ambiguous.`); | ||
} | ||
} | ||
|
||
return fixHints; | ||
} | ||
|
||
function extractKeyValuePairs(jsonLikeStr: string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have the relevant information accessible via AST nodes, so I don't think we should parse the argument ourselves here. The input is based on |
||
const regex = /["']?([\w$]+)["']?\s*:\s*(true|false|null|["'][^"']*["']|[+,\-./0-9:;<=>?@A-Z\\[\\\\]^_`a-e\]+)/g; | ||
const pairs = {} as Record<string, unknown>; | ||
let match; | ||
while ((match = regex.exec(jsonLikeStr)) !== null) { | ||
const key = match[1]; | ||
const rawValue = match[2]; | ||
let value; | ||
|
||
// Convert to appropriate JS type | ||
if (/^["'].*["']$/.test(rawValue)) { | ||
value = rawValue.slice(1, -1); // remove quotes | ||
} else if (rawValue === "true") { | ||
value = true; | ||
} else if (rawValue === "false") { | ||
value = false; | ||
} else if (rawValue === "null") { | ||
value = null; | ||
} else if (!isNaN(Number(rawValue))) { | ||
value = parseFloat(rawValue); | ||
} else { | ||
value = rawValue; // fallback | ||
} | ||
|
||
pairs[key] = value; | ||
} | ||
return pairs; | ||
} | ||
|
||
function cleanRedundantArguments(availableArgs: {value: string}[]) { | ||
const args = []; | ||
for (let i = 1; i <= availableArgs.length; i++) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,7 +147,9 @@ export default class TypeLinter { | |
continue; | ||
} | ||
let manifestContent; | ||
if (sourceFile.fileName.endsWith("/Component.js") || sourceFile.fileName.endsWith("/Component.ts")) { | ||
if (sourceFile.fileName.endsWith("/Component.js") || sourceFile.fileName.endsWith("/Component.ts") || | ||
// Manifest contains information that is needed for autofixing | ||
applyAutofix) { | ||
const res = await this.#workspace.byPath(path.dirname(sourceFile.fileName) + "/manifest.json"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if the current Please add a test-case for that (e.g. within the existing application project) |
||
if (res) { | ||
manifestContent = await res.getString(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using back-ticks as first arg (library name) results into quoted backticks.