Repository: platers/obsidian-linter (ts)

Issue:
Disable all doesnt disable custom rules
## Describe the Bug

Using the yaml key to disable all rules does not disable custom rules 

```
---
disabled rules: [ all ]
---
```

## How to Reproduce
1. Use custom rules
2. Disable all rules via yaml frontmatter
3. lint file



Pull request:
Have Custom Replacement Respect Ranged Ignores and YAML Fronmatter Disabling and Make Custom Commands not Run if `all` is Disable in Frontmatter
Fixes #739 

Changes Made:
- Updated the way UTs are set up for running tests for `rules-runner`
- Added UTs for custom replacement to make it more reliable and trustworthy
- Added logic to getting the list of disabled rules to also return whether or not to skip the current file (i.e. if `all` is present in the list of commands to ignore)
- Added logic to the rules runner to keep track of whether or not a particular file is to be ignored and not run custom commands on it if that is the case
- Updated custom replacement logic to respect ranged ignores
- Renamed proper ellipsis rule tests since it had `tests` in the name rather than `test`

Patch:
diff --git a/src/rules-runner.ts b/src/rules-runner.ts
index 96de3c7..b145921 100644
--- a/src/rules-runner.ts
+++ b/src/rules-runner.ts
@@ -22,6 +22,7 @@ import {convertStringVersionOfEscapeCharactersToEscapeCharacters} from './utils/
 import {getTextInLanguage} from './lang/helpers';
 import CapitalizeHeadings from './rules/capitalize-headings';
 import BlockquoteStyle from './rules/blockquote-style';
+import {IgnoreTypes, ignoreListOfTypes} from './utils/ignore-types';
 
 export type RunLinterRulesOptions = {
   oldText: string,
@@ -39,11 +40,17 @@ type FileInfo = {
 
 export class RulesRunner {
   private disabledRules: string[] = [];
+  skipFile: boolean;
 
   lintText(runOptions: RunLinterRulesOptions): string {
-    timingBegin(getTextInLanguage('logs.rule-running'));
+    this.skipFile = false;
     const originalText = runOptions.oldText;
-    this.disabledRules = getDisabledRules(originalText);
+    [this.disabledRules, this.skipFile] = getDisabledRules(originalText);
+    if (this.skipFile) {
+      return originalText;
+    }
+
+    timingBegin(getTextInLanguage('logs.rule-running'));
 
     const preRuleText = getTextInLanguage('logs.pre-rules');
     timingBegin(preRuleText);
@@ -135,6 +142,10 @@ export class RulesRunner {
   }
 
   runCustomCommands(lintCommands: LintCommand[], commands: ObsidianCommandInterface) {
+    if (this.skipFile) {
+      return;
+    }
+
     logDebug(getTextInLanguage('logs.running-custom-lint-command'));
     const commandsRun = new Set<string>();
     for (const commandInfo of lintCommands) {
@@ -155,18 +166,24 @@ export class RulesRunner {
   }
 
   runCustomRegexReplacement(customRegexes: CustomReplace[], oldText: string): string {
-    logDebug(getTextInLanguage('logs.running-custom-regex'));
-    let tempOldText = oldText;
-    for (const eachRegex of customRegexes) {
-      if (eachRegex.find == undefined || eachRegex.replace === undefined || eachRegex.replace === null ) {
-        continue;
+    return ignoreListOfTypes([IgnoreTypes.customIgnore], oldText, (text: string) => {
+      logDebug(getTextInLanguage('logs.running-custom-regex'));
+
+      let newText = text;
+      for (const eachRegex of customRegexes) {
+        const findIsEmpty = eachRegex.find === undefined || eachRegex.find == '' || eachRegex.find === null;
+        const replaceIsEmpty = eachRegex.replace === undefined || eachRegex.replace === null;
+        if (findIsEmpty || replaceIsEmpty) {
+          continue;
+        }
+
+        const regex = new RegExp(`${eachRegex.find}`, eachRegex.flags);
+        // make sure that characters are not string escaped unescape in the replace value to make sure things like \n and \t are correctly inserted
+        newText = newText.replace(regex, convertStringVersionOfEscapeCharactersToEscapeCharacters(eachRegex.replace));
       }
 
-      const regex = new RegExp(`${eachRegex.find}`, eachRegex.flags);
-      // make sure that characters are not string escaped unescape in the replace value to make sure things like \n and \t are correctly inserted
-      tempOldText = tempOldText.replace(regex, convertStringVersionOfEscapeCharactersToEscapeCharacters(eachRegex.replace));
-    }
-    return tempOldText;
+      return newText;
+    });
   }
 
   runPasteLint(currentLine: string, runOptions: RunLinterRulesOptions): string {
diff --git a/src/rules.ts b/src/rules.ts
index 6b79c44..8056b2a 100644
--- a/src/rules.ts
+++ b/src/rules.ts
@@ -163,18 +163,18 @@ export const RuleTypeOrder = Object.values(RuleType);
 /**
  * Returns a list of ignored rules in the YAML frontmatter of the text.
  * @param {string} text The text to parse
- * @return {string[]} The list of ignored rules
+ * @return {[string[], boolean]} The list of ignored rules and whether the current file should be ignored entirely
  */
-export function getDisabledRules(text: string): string[] {
+export function getDisabledRules(text: string): [string[], boolean] {
   const yaml = text.match(yamlRegex);
   if (!yaml) {
-    return [];
+    return [[], false];
   }
 
   const yaml_text = yaml[1];
   const disabledRulesValue = getYamlSectionValue(yaml_text, 'disabled rules');
   if (disabledRulesValue == null) {
-    return [];
+    return [[], false];
   }
 
   let disabledRulesKeyAndValue = disabledRulesValue.includes('\n') ? 'disabled rules:\n' : 'disabled rules: ';
@@ -185,7 +185,7 @@ export function getDisabledRules(text: string): string[] {
       'disabled rules'
   ];
   if (!disabled_rules) {
-    return [];
+    return [[], false];
   }
 
   if (typeof disabled_rules === 'string') {
@@ -193,10 +193,10 @@ export function getDisabledRules(text: string): string[] {
   }
 
   if (disabled_rules.includes('all')) {
-    return rules.map((rule) => rule.alias);
+    return [rules.map((rule) => rule.alias), true];
   }
 
-  return disabled_rules;
+  return [disabled_rules, false];
 }
 
 export const rules: Rule[] = [];


Test patch:
diff --git a/__tests__/disabled-rules.test.ts b/__tests__/disabled-rules.test.ts
index 199505a..9d5f880 100644
--- a/__tests__/disabled-rules.test.ts
+++ b/__tests__/disabled-rules.test.ts
@@ -5,6 +5,7 @@ type disabledRulesTestCase = {
   name: string,
   text: string,
   expectedDisabledRules: string[],
+  expectedSkipFileResult: boolean,
 };
 
 const disabledRulesTestCases: disabledRulesTestCase[] = [
@@ -14,6 +15,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: [],
+    expectedSkipFileResult: false,
   },
   {
     name: 'when there is no YAML key for disabled rules, no rules are disabled',
@@ -23,6 +25,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: [],
+    expectedSkipFileResult: false,
   },
   {
     name: 'when there is no value in YAML key for disabled rules, no rules are disabled',
@@ -33,6 +36,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: [],
+    expectedSkipFileResult: false,
   },
   {
     name: 'when there is one value in YAML key for disabled rules, that one rules is disabled',
@@ -43,6 +47,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: ['yaml-timestamp'],
+    expectedSkipFileResult: false,
   },
   {
     name: 'when there is more than one value in YAML key for disabled rules, those rules are disabled',
@@ -53,6 +58,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: ['yaml-timestamp', 'capitalize-headings'],
+    expectedSkipFileResult: false,
   },
   {
     name: 'when the value in YAML key for disabled rules is "all", all rules are disabled',
@@ -63,6 +69,7 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       Text
     `,
     expectedDisabledRules: rules.map((r) => r.alias),
+    expectedSkipFileResult: true,
   },
   {
     name: 'when the YAML is malformed, no error occurs trying to get disabled rules',
@@ -74,15 +81,17 @@ const disabledRulesTestCases: disabledRulesTestCase[] = [
       ---
     `,
     expectedDisabledRules: [],
+    expectedSkipFileResult: false,
   },
 ];
 
 describe('Disabled rules parsing', () => {
   for (const testCase of disabledRulesTestCases) {
     it(testCase.name, () => {
-      const disabledRules = getDisabledRules(testCase.text);
+      const [disabledRules, shouldSkipFile] = getDisabledRules(testCase.text);
 
       expect(disabledRules).toEqual(testCase.expectedDisabledRules);
+      expect(shouldSkipFile).toEqual(testCase.expectedSkipFileResult);
     });
   }
 });
diff --git a/__tests__/proper-ellipsis.tests.ts b/__tests__/proper-ellipsis.test.ts
similarity index 100%
rename from __tests__/proper-ellipsis.tests.ts
rename to __tests__/proper-ellipsis.test.ts
diff --git a/__tests__/rules-runner.test.ts b/__tests__/rules-runner.test.ts
index 47f30aa..5a53ff5 100644
--- a/__tests__/rules-runner.test.ts
+++ b/__tests__/rules-runner.test.ts
@@ -1,5 +1,7 @@
 import {Command} from 'obsidian';
 import {RulesRunner} from '../src/rules-runner';
+import {CustomReplace} from '../src/ui/linter-components/custom-replace-option';
+import dedent from 'ts-dedent';
 
 const rulesRunner = new RulesRunner();
 const appCommandsMock = {
@@ -27,47 +29,208 @@ const appCommandsMock = {
   },
 };
 
-afterEach(() => {
-  appCommandsMock.resetStats();
-});
-
-describe('Rules Runner', () => {
-  it('No app lint commands running should include no hits results for command lints run', () => {
-    rulesRunner.runCustomCommands([], appCommandsMock);
-
-    expect(appCommandsMock.numberOfCommands).toEqual(0);
-  });
+type CustomCommandTestCase = {
+  testName: string,
+  listOfCommands: Command[],
+  expectedCommandCount: Map<string, number>;
+  expectedNumberOfCommandsRun: number;
+  skipFileValue: boolean
+}
 
-  it('When an app lint command is run it should be executed', () => {
-    const listOfCommands = [
+const customCommandTestCases: CustomCommandTestCase[] = [
+  {
+    testName: 'No app lint commands running should include no hit results for command lint run',
+    listOfCommands: [],
+    expectedCommandCount: new Map<string, number>(),
+    expectedNumberOfCommandsRun: 0,
+    skipFileValue: false,
+  },
+  {
+    testName: 'When an app lint command is run it should be executed',
+    listOfCommands: [
+      {id: 'first id', name: 'command name'},
+      {id: 'second id', name: 'command name 2'},
+    ],
+    expectedCommandCount: new Map([
+      ['first id', 1],
+      ['second id', 1],
+    ]),
+    expectedNumberOfCommandsRun: 2,
+    skipFileValue: false,
+  },
+  {
+    testName: 'A lint command with an empty id should not get run',
+    listOfCommands: [
+      {id: '', name: ''},
+    ],
+    expectedCommandCount: new Map([
+      ['', 0],
+    ]),
+    expectedNumberOfCommandsRun: 0,
+    skipFileValue: false,
+  },
+  {
+    testName: 'When custom commands are run with two of the same command, the second command instance is skipped',
+    listOfCommands: [
+      {id: 'first id', name: 'command name'},
+      {id: 'first id', name: 'command name'},
+    ],
+    expectedCommandCount: new Map([
+      ['first id', 1],
+    ]),
+    expectedNumberOfCommandsRun: 1,
+    skipFileValue: false,
+  },
+  {
+    testName: 'When the file is listed to be skipped, no custom commands are run',
+    listOfCommands: [
       {id: 'first id', name: 'command name'},
       {id: 'second id', name: 'command name 2'},
-    ];
-    rulesRunner.runCustomCommands(listOfCommands, appCommandsMock);
+    ],
+    expectedCommandCount: new Map<string, number>(),
+    expectedNumberOfCommandsRun: 0,
+    skipFileValue: true,
+  },
+];
 
-    expect(appCommandsMock.numberOfCommands).toEqual(2);
-    expect(appCommandsMock.numberOfHitsPerId.get('first id') ?? 0).toEqual(1);
-    expect(appCommandsMock.numberOfHitsPerId.get('second id') ?? 0).toEqual(1);
-  });
 
-  it('A lint command with an empty id should not get run', () => {
-    const listOfCommands = [
-      {id: '', name: ''},
-    ];
-    rulesRunner.runCustomCommands(listOfCommands, appCommandsMock);
+type CustomReplaceTestCase = {
+  testName: string,
+  listOfRegexReplacements: CustomReplace[],
+  before: string,
+  after: string,
+}
 
-    expect(appCommandsMock.numberOfCommands).toEqual(0);
-    expect(appCommandsMock.numberOfHitsPerId.get('') ?? 0).toEqual(0);
-  });
+const customReplaceTestCases: CustomReplaceTestCase[] = [
+  {
+    testName: 'A custom replace with no find value does not affect the text',
+    listOfRegexReplacements: [
+      {
+        find: '', replace: 'hello', flags: 'g',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+    after: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+  },
+  {
+    testName: 'A custom replace with a null or undefined find value does not affect the text',
+    listOfRegexReplacements: [
+      {
+        find: 'How', replace: null, flags: '',
+      },
+      {
+        find: 'look', replace: undefined, flags: '',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+    after: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+  },
+  {
+    testName: 'A custom replace searching for multiple blank lines in a row works (has proper escaping of a slash)',
+    listOfRegexReplacements: [
+      {
+        find: '\n{3,}', replace: '\n\n', flags: 'g',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      ${''}
+      ${''}
+      Did it stay the same?
+    `,
+    after: dedent`
+      How does this look?
+      ${''}
+      Did it stay the same?
+    `,
+  },
+  {
+    testName: 'A custom replace using capture groups works',
+    listOfRegexReplacements: [
+      {
+        find: '(k|e)(\\?)', replace: '$1', flags: 'g',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+    after: dedent`
+      How does this look
+      Did it stay the same
+    `,
+  },
+  {
+    testName: 'A custom replace using ^ and $ works',
+    listOfRegexReplacements: [
+      {
+        find: '(^Did)|(look\\?$)', replace: 'swapped', flags: 'gm',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      Did it stay the same?
+    `,
+    after: dedent`
+      How does this swapped
+      swapped it stay the same?
+    `,
+  },
+  { // accounts for https://github.com/platers/obsidian-linter/issues/739
+    testName: 'A custom replace should respect linter ignore ranges',
+    listOfRegexReplacements: [
+      {
+        find: '(^Did)|(look\\?$)', replace: 'swapped', flags: 'gm',
+      },
+    ],
+    before: dedent`
+      How does this look?
+      <!-- linter-disable -->
+      Did it stay the same?
+      <!-- linter-enable -->
+    `,
+    after: dedent`
+      How does this swapped
+      <!-- linter-disable -->
+      Did it stay the same?
+      <!-- linter-enable -->
+    `,
+  },
+];
 
-  it('When custom commands are run with two of the same command, the second command instance is skipped', () => {
-    const listOfCommands = [
-      {id: 'first id', name: 'command name'},
-      {id: 'first id', name: 'command name'},
-    ];
-    rulesRunner.runCustomCommands(listOfCommands, appCommandsMock);
+describe('Rules Runner', () => {
+  // custom commands
+  for (const testCase of customCommandTestCases) {
+    it(testCase.testName, () => {
+      appCommandsMock.resetStats();
+      rulesRunner.skipFile = testCase.skipFileValue;
+      rulesRunner.runCustomCommands(testCase.listOfCommands, appCommandsMock);
+
+      expect(appCommandsMock.numberOfCommands).toEqual(testCase.expectedNumberOfCommandsRun);
+      for (const command of testCase.listOfCommands) {
+        expect(appCommandsMock.numberOfHitsPerId.get(command.id) ?? 0).toEqual(testCase.expectedCommandCount.get(command.id) ?? 0);
+      }
+    });
+  }
+
+  // custom regex replacement
+  for (const testCase of customReplaceTestCases) {
+    it(testCase.testName, () => {
+      const updateText = rulesRunner.runCustomRegexReplacement(testCase.listOfRegexReplacements, testCase.before);
 
-    expect(appCommandsMock.numberOfCommands).toEqual(1);
-    expect(appCommandsMock.numberOfHitsPerId.get('first id') ?? 0).toEqual(1);
-  });
+      expect(updateText).toEqual(testCase.after);
+    });
+  }
 });