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

FIX: Resolve Alias Recursion Issue by Preventing Duplicate Rule Application #1649

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
16 changes: 11 additions & 5 deletions src/Alias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,13 @@ export function removeAlias(name: string): boolean {
return hadAlias;
}

export function clearAliases(): void {
Aliases.clear();
GlobalAliases.clear();
}

/**
* Returns the original string with any aliases substituted in.
* Aliases are only applied to "whole words", one level deep
* @param origCommand the original command string
*/
export function substituteAliases(origCommand: string): string {
Expand Down Expand Up @@ -93,16 +97,18 @@ function applyAliases(origCommand: string, depth = 0, currentlyProcessingAliases
// First get non-global aliases, and recursively apply them
// (unless there are any reference loops or the reference chain is too deep)
const localAlias = Aliases.get(commandArray[0]);
if (localAlias && !currentlyProcessingAliases.includes(localAlias)) {
const appliedAlias = applyAliases(localAlias, depth + 1, [commandArray[0], ...currentlyProcessingAliases]);
const localrule = commandArray[0] + "->" + localAlias + "(l)";
if (localAlias && !currentlyProcessingAliases.includes(localrule)) {
const appliedAlias = applyAliases(localAlias, depth + 1, [localrule, ...currentlyProcessingAliases]);
cmfrydos marked this conversation as resolved.
Show resolved Hide resolved
commandArray.splice(0, 1, ...appliedAlias.split(" "));
}

// Once local aliasing is complete (or if none are present) handle any global aliases
const processedCommands = commandArray.reduce((resolvedCommandArray: string[], command) => {
const globalAlias = GlobalAliases.get(command);
if (globalAlias && !currentlyProcessingAliases.includes(globalAlias)) {
const appliedAlias = applyAliases(globalAlias, depth + 1, [command, ...currentlyProcessingAliases]);
const rule = command + "->" + globalAlias + "(g)";
if (globalAlias && !currentlyProcessingAliases.includes(rule)) {
const appliedAlias = applyAliases(globalAlias, depth + 1, [rule, ...currentlyProcessingAliases]);
resolvedCommandArray.push(appliedAlias);
} else {
// If there is no alias, or if the alias has a circular reference, leave the command as-is
Expand Down
51 changes: 46 additions & 5 deletions test/jest/Alias/Alias.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,54 @@
import { substituteAliases, parseAliasDeclaration } from "../../../src/Alias";
import { substituteAliases, parseAliasDeclaration, clearAliases } from "../../../src/Alias";
describe("substituteAliases Tests", () => {
it("Should gracefully handle recursive local aliases", () => {
beforeEach(() => {
clearAliases();
});
it("Should gracefully handle recursive local aliases I", () => {
parseAliasDeclaration("recursiveAlias=b");
parseAliasDeclaration("b=c");
parseAliasDeclaration("c=d");
parseAliasDeclaration("d=recursiveAlias");

const result = substituteAliases("recursiveAlias");
expect(result).toEqual("d");
expect(result).toEqual("recursiveAlias");
});

it("Should gracefully handle recursive local aliases II", () => {
parseAliasDeclaration("recursiveAlias=recursiveAlias");
const result = substituteAliases("recursiveAlias");
expect(result).toEqual("recursiveAlias");
});

it("Should gracefully handle recursive local aliases III", () => {
parseAliasDeclaration('recursiveAlias="recursiveAlias"');
const result = substituteAliases("recursiveAlias");
expect(result).toEqual("recursiveAlias");
});

it("Should gracefully handle recursive local aliases IV", () => {
parseAliasDeclaration('recursiveAlias="recursiveAlias -l"');
const result = substituteAliases("recursiveAlias");
expect(result).toEqual("recursiveAlias -l");
});

it("Should not substitute quoted commands I", () => {
parseAliasDeclaration("a=b");
const result = substituteAliases('"a"');
expect(result).toEqual('"a"');
});

it("Should not substitute quoted commands II", () => {
parseAliasDeclaration("a=b");
const result = substituteAliases("'a'");
expect(result).toEqual("'a'");
});

it("Should not substitute quoted commands III", () => {
parseAliasDeclaration("a=b");
parseAliasDeclaration("b='c'");
parseAliasDeclaration("c=d");
const result = substituteAliases("a");
//expect(result).toEqual("'c'"); // Currently FAILS
cmfrydos marked this conversation as resolved.
Show resolved Hide resolved
});

it("Should only change local aliases if they are the start of the command", () => {
Expand All @@ -27,7 +68,7 @@ describe("substituteAliases Tests", () => {
parseAliasDeclaration("d=a", true);

const result = substituteAliases("a b c d");
expect(result).toEqual("d a b c");
expect(result).toEqual("a b c d");
});

it("Should gracefully handle recursive mixed local and global aliases", () => {
Expand All @@ -37,7 +78,7 @@ describe("substituteAliases Tests", () => {
parseAliasDeclaration("d=recursiveAlias", false);

const result = substituteAliases("recursiveAlias");
expect(result).toEqual("d");
expect(result).toEqual("recursiveAlias");
});

it("Should replace chained aliases", () => {
Expand Down