Skip to content

Commit

Permalink
feat: add XML validation and quickfix for hardcoded UI text
Browse files Browse the repository at this point in the history
This is a validator that warns the user of hardcoded UI texts that could be externalized to a
resource bundle. Includes a quick fix for the warning, where externalized strings from the project's
i18n.properties file (if existent) may be suggested as replacements.
  • Loading branch information
fausto-m committed Sep 6, 2022
1 parent 9d0c89c commit 5c5968d
Show file tree
Hide file tree
Showing 23 changed files with 1,653 additions and 4 deletions.
16 changes: 16 additions & 0 deletions packages/language-server/src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { commands } from "@ui5-language-assistant/user-facing-text";
import {
executeQuickFixStableIdCommand,
executeQuickFixFileStableIdCommand,
executeQuickFixHardcodedI18nStringCommand,
} from "./quick-fix";
import { track } from "./swa";

Expand Down Expand Up @@ -44,6 +45,21 @@ export function executeCommand(
track("MANIFEST_STABLE_ID", "multiple");
return;
}
case commands.QUICK_FIX_HARDCODED_I18N_STRING_ERROR.name: {
const change = executeQuickFixHardcodedI18nStringCommand({
// Assumption that this command has the following arguments.
// We passed them when the command was created.
documentUri: params.arguments[0],
documentVersion: params.arguments[1],
quickFixReplaceRange: params.arguments[2],
quickFixNewText: params.arguments[3],
});
connection.workspace.applyEdit({
documentChanges: change,
});
track("MANIFEST_HARDCODED_I18N_STRING", "single");
return;
}
default:
return undefined;
}
Expand Down
85 changes: 84 additions & 1 deletion packages/language-server/src/quick-fix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@ import {
} from "@ui5-language-assistant/xml-views-validation";
import { UI5SemanticModel } from "@ui5-language-assistant/semantic-model-types";
import { computeQuickFixStableIdInfo } from "@ui5-language-assistant/xml-views-quick-fix";
import { computeQuickFixHardcodedI18nStringInfo } from "@ui5-language-assistant/xml-views-quick-fix";
import {
validations,
commands,
} from "@ui5-language-assistant/user-facing-text";
import { LSPRangeToOffsetRange, offsetRangeToLSPRange } from "./range-utils";
import { Property } from "properties-file";

type QuickFixStableIdLSPInfo = {
newText: string;
Expand All @@ -32,7 +34,8 @@ type QuickFixStableIdLSPInfo = {
export function diagnosticToCodeActionFix(
document: TextDocument,
diagnostics: Diagnostic[],
ui5Model: UI5SemanticModel
ui5Model: UI5SemanticModel,
resourceBundle: Property[]
): CodeAction[] {
const documentText = document.getText();
// We prefer to parse the document again to avoid cache state handling
Expand All @@ -49,6 +52,16 @@ export function diagnosticToCodeActionFix(
ui5Model,
});
}
case validations.HARDCODED_I18N_STRING.code: {
// hardcoded i18n string
return computeCodeActionsForQuickFixHardcodedI18nString({
document,
xmlDocument: xmlDocAst,
hardcodedI18nStringDiagnostic: diagnostic,
ui5Model,
resourceBundle,
});
}
default:
return [];
}
Expand Down Expand Up @@ -191,3 +204,73 @@ export function executeQuickFixFileStableIdCommand(opts: {

return documentEdit;
}

function computeCodeActionsForQuickFixHardcodedI18nString(opts: {
document: TextDocument;
xmlDocument: XMLDocument;
hardcodedI18nStringDiagnostic: Diagnostic;
ui5Model: UI5SemanticModel;
resourceBundle: Property[];
}): CodeAction[] {
const codeActions: CodeAction[] = [];

const errorOffset = LSPRangeToOffsetRange(
opts.hardcodedI18nStringDiagnostic.range,
opts.document
);

const quickFixHardcodedI18nStringInfo = computeQuickFixHardcodedI18nStringInfo(
opts.xmlDocument,
[errorOffset],
opts.resourceBundle
);

const replaceRange = offsetRangeToLSPRange(
quickFixHardcodedI18nStringInfo[0].replaceRange,
opts.document
);

quickFixHardcodedI18nStringInfo[0].newTextSuggestions.forEach(
(suggestion) => {
const codeActionTitle =
commands.QUICK_FIX_HARDCODED_I18N_STRING_ERROR.title +
": " +
suggestion.suggestionValue +
" (" +
suggestion.suggestionKey +
")";
codeActions.push(
CodeAction.create(
codeActionTitle,
Command.create(
commands.QUICK_FIX_HARDCODED_I18N_STRING_ERROR.title,
commands.QUICK_FIX_HARDCODED_I18N_STRING_ERROR.name,
opts.document.uri,
opts.document.version,
replaceRange,
suggestion.newText
),
CodeActionKind.QuickFix
)
);
}
);

return codeActions;
}

export function executeQuickFixHardcodedI18nStringCommand(opts: {
documentUri: string;
documentVersion: number;
quickFixReplaceRange: LSPRange;
quickFixNewText: string;
}): TextDocumentEdit[] {
const documentEdit = [
TextDocumentEdit.create(
{ uri: opts.documentUri, version: opts.documentVersion },
[TextEdit.replace(opts.quickFixReplaceRange, `${opts.quickFixNewText}`)]
),
];

return documentEdit;
}
122 changes: 122 additions & 0 deletions packages/language-server/src/resource-bundle-handling.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { dirname } from "path";
import { maxBy, map, filter } from "lodash";
import { readFile } from "fs-extra";
import { URI } from "vscode-uri";
import globby from "globby";
import { FileChangeType } from "vscode-languageserver";
import { getLogger } from "./logger";
import * as propertiesParser from "properties-file/content";
import { Property } from "properties-file";

type AbsolutePath = string;
type ResourceBundleData = Record<AbsolutePath, Property[]>;
const resourceBundleData: ResourceBundleData = Object.create(null);

export function isResourceBundleDoc(uri: string): boolean {
return uri.endsWith("i18n.properties");
}

export async function initializeResourceBundleData(
workspaceFolderPath: string
): Promise<void[]> {
const resourceBundleDocuments = await findAllResourceBundleDocumentsInWorkspace(
workspaceFolderPath
);

const readResourceBundlePromises = map(
resourceBundleDocuments,
async (resourceBundleDoc) => {
const response = await readResourceBundleFile(resourceBundleDoc);

// Parsing of i18n.properties failed because the file is invalid
if (response !== "INVALID") {
resourceBundleData[resourceBundleDoc] = response;
}
}
);

getLogger().info("resourceBundle data initialized", {
resourceBundleDocuments,
});
return Promise.all(readResourceBundlePromises);
}

export function getResourceBundleData(documentPath: string): Property[] {
const resourceBundleFilesForCurrentFolder = filter(
Object.keys(resourceBundleData),
(resourceBundlePath) =>
documentPath.startsWith(dirname(resourceBundlePath.replace("/i18n", "")))
);

const closestResourceBundlePath = maxBy(
resourceBundleFilesForCurrentFolder,
(resourceBundlePath) => resourceBundlePath.length
);

if (closestResourceBundlePath === undefined) {
return [];
}

return resourceBundleData[closestResourceBundlePath];
}

export async function updateResourceBundleData(
resourceBundleUri: string,
changeType: FileChangeType
): Promise<void> {
getLogger().debug("`updateResourceBundleData` function called", {
resourceBundleUri,
changeType,
});
const resourceBundlePath = URI.parse(resourceBundleUri).fsPath;
switch (changeType) {
case 1: //created
case 2: {
//changed
const response = await readResourceBundleFile(resourceBundleUri);
// Parsing of i18n.properties failed because the file is invalid
// We want to keep last successfully read state - i18n.properties file may be actively edited
if (response !== "INVALID") {
resourceBundleData[resourceBundlePath] = response;
}
return;
}
case 3: //deleted
delete resourceBundleData[resourceBundlePath];
return;
}
}

async function findAllResourceBundleDocumentsInWorkspace(
workspaceFolderPath: string
): Promise<string[]> {
return globby(`${workspaceFolderPath}/**/i18n.properties`).catch((reason) => {
getLogger().error(
`Failed to find all i18n.properties files in current workspace!`,
{
workspaceFolderPath,
reason,
}
);
return [];
});
}

async function readResourceBundleFile(
resourceBundleUri: string
): Promise<Property[] | "INVALID"> {
const resourceBundleContent = await readFile(
URI.parse(resourceBundleUri).fsPath,
"utf-8"
);

let resourceBundleObject: Property[];
try {
resourceBundleObject = propertiesParser.getProperties(resourceBundleContent)
.collection;
} catch (err) {
return "INVALID";
}

return resourceBundleObject;
}
24 changes: 22 additions & 2 deletions packages/language-server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ import {
initializeManifestData,
updateManifestData,
} from "./manifest-handling";
import {
getResourceBundleData,
initializeResourceBundleData,
isResourceBundleDoc,
updateResourceBundleData,
} from "./resource-bundle-handling";
import {
getUI5FrameworkForXMLFile,
isUI5YamlDoc,
Expand All @@ -50,6 +56,7 @@ const connection = createConnection(ProposedFeatures.all);
const documents = new TextDocuments(TextDocument);
let manifestStateInitialized: Promise<void[]> | undefined = undefined;
let ui5yamlStateInitialized: Promise<void[]> | undefined = undefined;
let resourceBundletStateInitialized: Promise<void[]> | undefined = undefined;
let initializationOptions: ServerInitializationOptions | undefined;
let hasConfigurationCapability = false;

Expand All @@ -66,6 +73,9 @@ connection.onInitialize((params: InitializeParams) => {
const workspaceFolderAbsPath = URI.parse(workspaceFolderUri).fsPath;
manifestStateInitialized = initializeManifestData(workspaceFolderAbsPath);
ui5yamlStateInitialized = initializeUI5YamlData(workspaceFolderAbsPath);
resourceBundletStateInitialized = initializeResourceBundleData(
workspaceFolderAbsPath
);
}

// Does the client support the `workspace/configuration` request?
Expand All @@ -92,6 +102,7 @@ connection.onInitialize((params: InitializeParams) => {
commands: [
commands.QUICK_FIX_STABLE_ID_ERROR.name,
commands.QUICK_FIX_STABLE_ID_FILE_ERRORS.name,
commands.QUICK_FIX_HARDCODED_I18N_STRING_ERROR.name,
],
},
},
Expand Down Expand Up @@ -198,6 +209,8 @@ connection.onDidChangeWatchedFiles(async (changeEvent) => {
await updateManifestData(uri, change.type);
} else if (isUI5YamlDoc(uri)) {
await updateUI5YamlData(uri, change.type);
} else if (isResourceBundleDoc(uri)) {
await updateResourceBundleData(uri, change.type);
}
});
});
Expand All @@ -207,12 +220,17 @@ documents.onDidChangeContent(async (changeEvent) => {
if (
manifestStateInitialized === undefined ||
ui5yamlStateInitialized === undefined ||
resourceBundletStateInitialized === undefined ||
!isXMLView(changeEvent.document.uri)
) {
return;
}

await Promise.all([manifestStateInitialized, ui5yamlStateInitialized]);
await Promise.all([
manifestStateInitialized,
ui5yamlStateInitialized,
resourceBundletStateInitialized,
]);
const documentUri = changeEvent.document.uri;
const document = documents.get(documentUri);
if (document !== undefined) {
Expand Down Expand Up @@ -263,11 +281,13 @@ connection.onCodeAction(async (params) => {
version: ui5Model.version,
});

const resourceBundle = getResourceBundleData(documentPath);
const diagnostics = params.context.diagnostics;
const codeActions = diagnosticToCodeActionFix(
textDocument,
diagnostics,
ui5Model
ui5Model,
resourceBundle
);
getLogger().trace("`computed codeActions", { codeActions });
return codeActions;
Expand Down
1 change: 1 addition & 0 deletions packages/language-server/src/swa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ export function initSwa(
export const TRACK_EVENTS = {
MANIFEST_STABLE_ID: "manifest stable ID fix",
XML_UI5_DOC_HOVER: "XML UI5 Doc Hover",
MANIFEST_HARDCODED_I18N_STRING: "hardcoded i18n string fix",
};

Object.freeze(TRACK_EVENTS);
Expand Down
5 changes: 5 additions & 0 deletions packages/language-server/src/xml-view-diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,11 @@ function validationIssuesToLspDiagnostics(
...commonDiagnosticPros,
code: validations.NON_STABLE_ID.code,
};
case "UseOfHardcodedI18nString":
return {
...commonDiagnosticPros,
code: validations.HARDCODED_I18N_STRING.code,
};
case "UseOfDeprecatedClass":
case "UseOfDeprecatedProperty":
case "UseOfDeprecatedEvent":
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<mvc:View xmlns:m="sap.m"
xmlns:mvc="sap.ui.core.mvc">
<m:Page>
<m:Button text=🢂"i18n_dummy_text"🢀/>
</m:Page>
</mvc:View>
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
[
{
"range": {
"start": { "line": 3, "character": 19 },
"end": { "line": 3, "character": 36 }
},
"severity": 2,
"source": "UI5 Language Assistant",
"message": "Consider externalizing UI texts to a resource bundle or other model: \"i18n_dummy_text\".",
"code": 1013
}
]
Loading

0 comments on commit 5c5968d

Please sign in to comment.