Skip to content

feat: Autofix deprecated class properties in XML #660

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 43 additions & 7 deletions src/autofix/autofix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {Resource} from "@ui5/fs";
import {collectIdentifiers} from "./utils.js";
import {ExportCodeToBeUsed} from "../linter/ui5Types/fixHints/FixHints.js";
import generateSolutionCodeReplacer from "./solutions/codeReplacer.js";
import generateXmlSolutionDeprecatedApi from "./solutions/generateXmlSolutionDeprecatedApi.js";

const log = getLogger("linter:autofix");

Expand Down Expand Up @@ -215,22 +216,27 @@ export default async function ({
}: AutofixOptions): Promise<AutofixResult> {
// Group messages by ID and only process files for which fixes are available
const messages = new Map<string, Map<MESSAGE, RawLintMessage[]>>();
const resources: Resource[] = [];
const jsResources: Resource[] = [];
const xmlResources: Resource[] = [];
for (const [_, autofixResource] of autofixResources) {
const messagesById = getAutofixMessages(autofixResource);
// Currently only global access autofixes are supported
// This needs to stay aligned with the applyFixes function
// Set of supported message IDs needs to stay aligned with the applyFixes function
if (messagesById.has(MESSAGE.NO_GLOBALS) ||
messagesById.has(MESSAGE.DEPRECATED_API_ACCESS) ||
messagesById.has(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS) ||
messagesById.has(MESSAGE.DEPRECATED_FUNCTION_CALL)) {
messages.set(autofixResource.resource.getPath(), messagesById);
resources.push(autofixResource.resource);
if (autofixResource.resource.getPath().endsWith(".xml")) {
xmlResources.push(autofixResource.resource);
} else {
jsResources.push(autofixResource.resource);
}
}
}

const sourceFiles: SourceFiles = new Map();
const resourcePaths = [];
for (const resource of resources) {
for (const resource of jsResources) {
const resourcePath = resource.getPath();
const sourceFile = ts.createSourceFile(
resource.getPath(),
Expand Down Expand Up @@ -262,7 +268,7 @@ export default async function ({
log.verbose(`Applying autofixes to ${resourcePath}`);
let newContent;
try {
newContent = applyFixes(checker, sourceFile, resourcePath, messages.get(resourcePath)!);
newContent = applyFixesJs(checker, sourceFile, resourcePath, messages.get(resourcePath)!);
} catch (err) {
if (err instanceof Error) {
log.verbose(`Error while applying autofix to ${resourcePath}: ${err}`);
Expand All @@ -286,10 +292,18 @@ export default async function ({
}
}

for (const resource of xmlResources) {
const resourcePath = resource.getPath();
const newContent = await applyFixesXml(resource, messages.get(resourcePath)!);
if (newContent) {
res.set(resourcePath, newContent);
}
}

return res;
}

function applyFixes(
function applyFixesJs(
checker: ts.TypeChecker, sourceFile: ts.SourceFile, resourcePath: ResourcePath,
messagesById: Map<MESSAGE, RawLintMessage[]>
): string | undefined {
Expand Down Expand Up @@ -353,6 +367,28 @@ function applyFixes(
return applyChanges(content, changeSet);
}

async function applyFixesXml(
resource: Resource,
messagesById: Map<MESSAGE, RawLintMessage[]>
): Promise<string | undefined> {
const changeSet: ChangeSet[] = [];
const messages: RawLintMessage<MESSAGE.DEPRECATED_PROPERTY_OF_CLASS>[] = [];

if (messagesById.has(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS)) {
messages.push(
...messagesById.get(
MESSAGE.DEPRECATED_PROPERTY_OF_CLASS) as RawLintMessage<MESSAGE.DEPRECATED_PROPERTY_OF_CLASS>[]
);
}
const content = await resource.getString();
await generateXmlSolutionDeprecatedApi(messages, changeSet, content, resource);

if (changeSet.length === 0) {
return undefined;
}
return applyChanges(content, changeSet);
}

function applyChanges(content: string, changeSet: ChangeSet[]): string {
changeSet.sort((a, b) => b.start - a.start);
const s = new MagicString(content);
Expand Down
67 changes: 67 additions & 0 deletions src/autofix/solutions/generateXmlSolutionDeprecatedApi.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {Resource} from "@ui5/fs";
import {Attribute, Position, SaxEventType} from "sax-wasm";
import {RawLintMessage} from "../../linter/LinterContext.js";
import {MESSAGE} from "../../linter/messages.js";
import {parseXml} from "../../utils/xmlParser.js";
import {ChangeAction, ChangeSet} from "../autofix.js";
// import {getLogger} from "@ui5/logger";

// const log = getLogger("linter:autofix:generateXmlSolutionDeprecatedApi");

export default async function generateXmlSolutionDeprecatedApi(
messages: RawLintMessage<MESSAGE.DEPRECATED_PROPERTY_OF_CLASS>[],
changeSet: ChangeSet[], content: string, resource: Resource) {
function toPosition(position: Position) {
let pos: number;
if (position.line === 0) {
pos = position.character;
} else {
pos = 0;
const lines = content.split("\n");
for (let i = 0; i < position.line; i++) {
pos += lines[i].length + 1; // +1 for the newline character we used to split the lines with
}
pos += position.character;
}
return pos;
}

function handleAttribute(attr: Attribute) {
// Check whether line and column match with any of the messages
const line = attr.name.start.line + 1;
const column = attr.name.start.character + 1;
const message = messages.find((message) => {
return message.position?.line === line && message.position?.column === column;
});
if (!message?.fixHints) {
return;
}

const {classProperty, classPropertyToBeUsed} = message.fixHints;

if (classProperty === undefined || classPropertyToBeUsed === undefined) {
return;
}

if (classPropertyToBeUsed) {
changeSet.push({
action: ChangeAction.REPLACE,
start: toPosition(attr.name.start),
end: toPosition(attr.name.end),
value: classPropertyToBeUsed,
});
} else {
changeSet.push({
action: ChangeAction.DELETE,
start: toPosition(attr.name.start),
end: toPosition(attr.value.end) + 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the +1 might be wrong if the attribute value is not within quotes

});
}
}

await parseXml(resource.getStream(), (event, tag) => {
if (event === SaxEventType.Attribute) {
handleAttribute(tag as Attribute);
}
}, SaxEventType.Attribute);
}
4 changes: 2 additions & 2 deletions src/linter/dotLibrary/DotLibraryLinter.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import LinterContext from "../LinterContext.js";
import {deprecatedLibraries} from "../../utils/deprecations.js";
import {SaxEventType, Tag as SaxTag} from "sax-wasm";
import {parseXML} from "../../utils/xmlParser.js";
import {parseXml} from "../../utils/xmlParser.js";
import {ReadStream} from "node:fs";
import {MESSAGE} from "../messages.js";

Expand Down Expand Up @@ -30,7 +30,7 @@ export default class DotLibraryLinter {
const libs = new Set();
const tagsStack: string[] = [];
const libNamePath = ["library", "dependencies", "dependency"];
await parseXML(contentStream, (event, tag) => {
await parseXml(contentStream, (event, tag) => {
if (!(tag instanceof SaxTag)) {
return;
}
Expand Down
4 changes: 2 additions & 2 deletions src/linter/html/parser.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import type {ReadStream} from "node:fs";
import {SaxEventType, Tag as SaxTag, Text as SaxText} from "sax-wasm";
import {extractDirective, parseXML} from "../../utils/xmlParser.js";
import {extractDirective, parseXml} from "../../utils/xmlParser.js";
import {Directive} from "../LinterContext.js";

interface ExtractedTags {
Expand Down Expand Up @@ -51,7 +51,7 @@ export async function extractHTMLTags(contentStream: ReadStream) {
stylesheetLinkTags: [],
};
const directives = new Set<Directive>();
await parseXML(contentStream, (event, tag) => {
await parseXml(contentStream, (event, tag) => {
if (tag instanceof SaxTag) {
parseTag(event, tag.toJSON() as SaxTag, extractedTags);
} else if (tag instanceof SaxText && event === SaxEventType.Comment) {
Expand Down
25 changes: 19 additions & 6 deletions src/linter/ui5Types/SourceFileLinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -701,12 +701,12 @@ export default class SourceFileLinter {
if (!ts.isPropertyAssignment(prop)) {
return;
}
const propertyName = getPropertyNameText(prop.name);
if (!propertyName) {
const propertyNameText = getPropertyNameText(prop.name);
if (!propertyNameText) {
return;
}
const propertySymbol = getSymbolForPropertyInConstructSignatures(
possibleConstructSignatures, argIdx, propertyName
possibleConstructSignatures, argIdx, propertyNameText
);
if (!propertySymbol) {
return;
Expand All @@ -715,13 +715,22 @@ export default class SourceFileLinter {
if (!deprecationInfo) {
return;
}

const propertyName = propertySymbol.escapedName as string;
const className = this.checker.typeToString(nodeType);
let fixHints;
if (moduleDeclaration?.name.text) {
fixHints = this.getDeprecatedClassPropertyFixHints(
prop, propertyName, moduleDeclaration.name.text);
}
this.#reporter.addMessage(MESSAGE.DEPRECATED_PROPERTY_OF_CLASS,
{
propertyName: propertySymbol.escapedName as string,
className: this.checker.typeToString(nodeType),
propertyName,
className,
details: deprecationInfo.messageDetails,
},
prop
prop,
fixHints
);
});
});
Expand Down Expand Up @@ -1812,4 +1821,8 @@ export default class SourceFileLinter {
getJquerySapFixHints(node: ts.CallExpression | ts.AccessExpression) {
return this.#fixHintsGenerator?.getJquerySapFixHints(node);
}

getDeprecatedClassPropertyFixHints(node: ts.PropertyAssignment, propertyName: string, className: string) {
return this.#fixHintsGenerator?.getDeprecatedClassPropertyFixHints(node, propertyName, className);
}
}
30 changes: 30 additions & 0 deletions src/linter/ui5Types/fixHints/DeprecatedClassPropertyGenerator.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import ts from "typescript";
import type {FixHints} from "./FixHints.js";

const CLASS_PROPERTY_REPLACEMENTS = new Map<string, Map<string, FixHints>>([
["sap/ui/comp/smarttable/SmartTable", new Map([
["useExportToExcel", {
classProperty: "useExportToExcel",
classPropertyToBeUsed: "enableExport",
}],
])],
["sap/ui/layout/form/SimpleForm", new Map([
["minWidth", {
classProperty: "minWidth",
classPropertyToBeUsed: "",
}],
])],
["sap/m/Button", new Map([
["tap", {
classProperty: "tap",
classPropertyToBeUsed: "press",
}],
])],
]);

export default class DeprprecatedClassPropertyGenerator {
getFixHints(node: ts.PropertyAssignment, propertyName: string, className: string): FixHints | undefined {
const fixHint = CLASS_PROPERTY_REPLACEMENTS.get(className)?.get(propertyName);
return fixHint;
}
}
3 changes: 3 additions & 0 deletions src/linter/ui5Types/fixHints/FixHints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,7 @@ export interface FixHints {
* e.g. `if (window.sap.ui.layout) { ... }`
*/
conditional?: boolean;

classProperty?: string;
classPropertyToBeUsed?: string;
}
9 changes: 9 additions & 0 deletions src/linter/ui5Types/fixHints/FixHintsGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,20 @@ import {AmbientModuleCache} from "../AmbientModuleCache.js";
import GlobalsFixHintsGenerator from "./GlobalsFixHintsGenerator.js";
import JquerySapFixHintsGenerator from "./JquerySapFixHintsGenerator.js";
import {FixHints} from "./FixHints.js";
import DeprecatedClassPropertyGenerator from "./DeprecatedClassPropertyGenerator.js";

export default class FixHintsGenerator {
private globalsGenerator: GlobalsFixHintsGenerator;
private jquerySapGenerator: JquerySapFixHintsGenerator;
private deprecatedClassPropertyGenerator: DeprecatedClassPropertyGenerator;

constructor(
resourcePath: string,
ambientModuleCache: AmbientModuleCache
) {
this.globalsGenerator = new GlobalsFixHintsGenerator(resourcePath, ambientModuleCache);
this.jquerySapGenerator = new JquerySapFixHintsGenerator();
this.deprecatedClassPropertyGenerator = new DeprecatedClassPropertyGenerator();
}

public getGlobalsFixHints(node: ts.CallExpression | ts.AccessExpression): FixHints | undefined {
Expand All @@ -25,4 +28,10 @@ export default class FixHintsGenerator {
): FixHints | undefined {
return this.jquerySapGenerator.getFixHints(node);
}

public getDeprecatedClassPropertyFixHints(
node: ts.PropertyAssignment, propertyName: string, className: string
): FixHints | undefined {
return this.deprecatedClassPropertyGenerator.getFixHints(node, propertyName, className);
}
}
Loading