Skip to content
This repository has been archived by the owner on Oct 10, 2018. It is now read-only.

Commit

Permalink
perf: Optimize and cleanup regex usage (#337)
Browse files Browse the repository at this point in the history
  • Loading branch information
tdd authored and buehler committed Nov 6, 2017
1 parent a04ce1e commit 06b739c
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/common/helpers/DeclarationIndexHelpers.ts
Expand Up @@ -60,7 +60,7 @@ export function getAbsolutLibraryName(library: string, actualFilePath: string, r
return '/' + toPosix(relative(
rootPath,
normalize(join(parse(actualFilePath).dir, library)),
)).replace(/[/]$/g, '');
)).replace(/\/$/, '');
}

/**
Expand Down Expand Up @@ -160,7 +160,7 @@ export async function findFiles(config: ExtensionConfig, workspaceFolder: Worksp
o.filter(
f => f.fsPath
.replace(rootPath || '', '')
.split(/\\|\//)
.split(/[\\/]/)
.every(p => excludePatterns.indexOf(p) < 0)) :
o,
);
Expand Down
4 changes: 3 additions & 1 deletion src/common/helpers/ImportHelpers.ts
@@ -1,5 +1,7 @@
import { Position, TextEditor } from 'vscode';

const REGEX_IGNORED_LINE = /^\s*(?:\/\/|\/\*\*|\*\/|\*|(['"])use strict\1)/;

/**
* Calculate the position, where a new import should be inserted.
* Does respect the "use strict" string as first line of a document.
Expand All @@ -13,6 +15,6 @@ export function getImportInsertPosition(editor: TextEditor | undefined): Positio
return new Position(0, 0);
}
const lines = editor.document.getText().split('\n');
const index = lines.findIndex(line => !line.match(/^\s*(\/\/|\/\*\*|\*\/|\*|['"]use strict['"])/g));
const index = lines.findIndex(line => !REGEX_IGNORED_LINE.test(line));
return new Position(Math.max(0, index), 0);
}
18 changes: 11 additions & 7 deletions src/extension/code-actions/MissingImplementationInClassCreator.ts
Expand Up @@ -9,6 +9,10 @@ import { Logger } from '../utilities/winstonLogger';
import { ImplementPolymorphElements, NoopCodeAction } from './CodeAction';
import { CodeActionCreator } from './CodeActionCreator';

const REGEX_GENERICS = /^(.*)<(.*)>$/;
const REGEX_INCORRECT_IMPL = /class (['"])(.*)\1 incorrectly implements.*(['"])(.*)\3\./i;
const REGEX_NON_ABSTRACT_IMPL = /non-abstract class (['"])(.*)\1.*implement inherited.*from class (['"])(.*)\3\./i;

/**
* Action creator that handles missing implementations in a class.
*
Expand All @@ -35,8 +39,8 @@ export class MissingImplementationInClassCreator extends CodeActionCreator {
* @memberof MissingImplementationInClassCreator
*/
public canHandleDiagnostic(diagnostic: Diagnostic): boolean {
return /class ['"](.*)['"] incorrectly implements.*['"](.*)['"]\./ig.test(diagnostic.message) ||
/non-abstract class ['"](.*)['"].*implement inherited.*from class ['"](.*)['"]\./ig.test(diagnostic.message);
return REGEX_INCORRECT_IMPL.test(diagnostic.message) ||
REGEX_NON_ABSTRACT_IMPL.test(diagnostic.message);
}

/**
Expand All @@ -50,8 +54,8 @@ export class MissingImplementationInClassCreator extends CodeActionCreator {
* @memberof MissingImplementationInClassCreator
*/
public async handleDiagnostic(document: TextDocument, commands: Command[], diagnostic: Diagnostic): Promise<Command[]> {
const match = /class ['"](.*)['"] incorrectly implements.*['"](.*)['"]\./ig.exec(diagnostic.message) ||
/non-abstract class ['"](.*)['"].*implement inherited.*from class ['"](.*)['"]\./ig.exec(diagnostic.message);
const match = REGEX_INCORRECT_IMPL.exec(diagnostic.message) ||
REGEX_NON_ABSTRACT_IMPL.exec(diagnostic.message);

const index = this.indices.getIndexForFile(document.uri);
const rootFolder = workspace.getWorkspaceFolder(document.uri);
Expand All @@ -64,10 +68,10 @@ export class MissingImplementationInClassCreator extends CodeActionCreator {
return commands;
}

let specifier = match[2];
let specifier = match[4];
let types: string[] | undefined;
let typeParams: { [type: string]: string } | undefined;
const genericMatch = /^(.*)[<](.*)[>]$/.exec(specifier);
const genericMatch = REGEX_GENERICS.exec(specifier);

if (genericMatch) {
specifier = genericMatch[1];
Expand Down Expand Up @@ -115,7 +119,7 @@ export class MissingImplementationInClassCreator extends CodeActionCreator {

commands.push(this.createCommand(
`Implement missing elements from "${genericMatch && types ? `${specifier}<${types.join(', ')}>` : specifier}".`,
new ImplementPolymorphElements(document, match[1], declaration, typeParams),
new ImplementPolymorphElements(document, match[2], declaration, typeParams),
));

this.logger.debug(
Expand Down
12 changes: 7 additions & 5 deletions src/extension/code-actions/MissingImportCreator.ts
Expand Up @@ -7,6 +7,8 @@ import { DeclarationIndexMapper } from '../utilities/DeclarationIndexMapper';
import { AddImportCodeAction, AddMissingImportsCodeAction, NoopCodeAction } from './CodeAction';
import { CodeActionCreator } from './CodeActionCreator';

const REGEX_CANNOT_FIND_IMPORT = /cannot find name (['"])(.*)\1/i;

/**
* Action creator that handles missing imports in files.
*
Expand All @@ -32,7 +34,7 @@ export class MissingImportCreator extends CodeActionCreator {
* @memberof MissingImportCreator
*/
public canHandleDiagnostic(diagnostic: Diagnostic): boolean {
return /cannot find name ['"](.*)['"]/ig.test(diagnostic.message);
return REGEX_CANNOT_FIND_IMPORT.test(diagnostic.message);
}

/**
Expand All @@ -46,7 +48,7 @@ export class MissingImportCreator extends CodeActionCreator {
* @memberof MissingImportCreator
*/
public async handleDiagnostic(document: TextDocument, commands: Command[], diagnostic: Diagnostic): Promise<Command[]> {
const match = /cannot find name ['"](.*)['"]/ig.exec(diagnostic.message);
const match = REGEX_CANNOT_FIND_IMPORT.exec(diagnostic.message);
const index = this.indices.getIndexForFile(document.uri);

if (!match || !index) {
Expand All @@ -57,7 +59,7 @@ export class MissingImportCreator extends CodeActionCreator {
return commands;
}

const infos = index.declarationInfos.filter(o => o.declaration.name === match[1]);
const infos = index.declarationInfos.filter(o => o.declaration.name === match[2]);
if (infos.length > 0) {
for (const info of infos) {
commands.push(this.createCommand(
Expand Down Expand Up @@ -87,13 +89,13 @@ export class MissingImportCreator extends CodeActionCreator {
}
} else {
commands.push(this.createCommand(
`Cannot find "${match[1]}" in the index.`,
`Cannot find "${match[2]}" in the index.`,
new NoopCodeAction(),
));
this.logger.debug(
'[%s] class not found in index',
MissingImportCreator.name,
{ specifier: match[1] },
{ specifier: match[2] },
);
}

Expand Down
6 changes: 4 additions & 2 deletions src/extension/extensions/CodeCompletionExtension.ts
Expand Up @@ -21,6 +21,8 @@ import { getItemKind } from '../utilities/utilityFunctions';
import { Logger } from '../utilities/winstonLogger';
import { BaseExtension } from './BaseExtension';

const REGEX_COMMENT = /^\s*(?:\/\/|\/\*\*|\*\/|\*)/;

/**
* Extension that provides code completion for typescript files. Uses the calculated index to provide information.
*
Expand Down Expand Up @@ -113,9 +115,9 @@ export class CodeCompletionExtension extends BaseExtension implements Completion
token.isCancellationRequested ||
!index.indexReady ||
(lineText.substring(0, position.character).match(/["'`]/g) || []).length % 2 === 1 ||
lineText.match(/^\s*(\/\/|\/\*\*|\*\/|\*)/g) ||
REGEX_COMMENT.test(lineText) ||
lineText.startsWith('import ') ||
lineText.substring(0, position.character).match(new RegExp(`(\w*[.])+${searchWord}`, 'g'))) {
new RegExp(`(?:\w*\.)+${searchWord}`).test(lineText.substring(0, position.character))) {
this.logger.debug(
'[%s] did not match criteria to provide intellisense',
CodeCompletionExtension.name,
Expand Down
14 changes: 8 additions & 6 deletions src/extension/import-grouping/ImportGroupSettingParser.ts
Expand Up @@ -11,10 +11,12 @@ import { RemainImportGroup } from './RemainImportGroup';
*/
export type ImportGroupSetting = string | { identifier: string, order: ImportGroupOrder };

const REGEX_REGEX_GROUP = /^\/.+\/$/;

/**
* Parser that takes the vscode - setting and creates import groups out of it.
* Contains a default if the parsing fails.
*
*
* @export
* @class ImportGroupSettingParser
*/
Expand All @@ -25,7 +27,7 @@ export class ImportGroupSettingParser {
* - Plain imports
* - Module imports
* - Workspace imports
*
*
* @readonly
* @static
* @type {ImportGroup[]}
Expand All @@ -42,12 +44,12 @@ export class ImportGroupSettingParser {

/**
* Function that takes a string or object ({@link ImportGroupSetting}) and parses an import group out of it.
*
*
* @static
* @param {ImportGroupSetting} setting
* @param {ImportGroupSetting} setting
* @returns {ImportGroup}
* @throws {ImportGroupIdentifierInvalidError} When the identifier is invalid (neither keyword nor valid regex)
*
*
* @memberof ImportGroupSettingParser
*/
public static parseSetting(setting: ImportGroupSetting): ImportGroup {
Expand All @@ -61,7 +63,7 @@ export class ImportGroupSettingParser {
order = setting.order;
}

if (/^\/.+.*\/$/g.test(identifier)) {
if (REGEX_REGEX_GROUP.test(identifier)) {
return new RegexImportGroup(identifier, order);
}

Expand Down
Expand Up @@ -53,7 +53,8 @@ describe('OrganizeImportsOnSaveExtension', () => {
});
});

it('should remove an unused import on save', async () => {
it('should remove an unused import on save', async function () {
this.timeout(4000);
const ctrl = await ImportManager.create(document);
const declaration = index.declarationInfos.find(o => o.declaration.name === 'Class1');
ctrl.addDeclarationImport(declaration!);
Expand All @@ -68,7 +69,8 @@ describe('OrganizeImportsOnSaveExtension', () => {
document.lineAt(0).text.should.equals('');
});

it('should not remove a used import on save', async () => {
it('should not remove a used import on save', async function () {
this.timeout(4000);
const ctrl = await ImportManager.create(document);
const declaration = index.declarationInfos.find(o => o.declaration.name === 'Class1');
const declaration2 = index.declarationInfos.find(o => o.declaration.name === 'Class2');
Expand Down

0 comments on commit 06b739c

Please sign in to comment.