Repository: platers/obsidian-linter (ts)

Issue:
Bug: Titles with special characters not getting escaped in aliases
## Describe the Bug

Titles containing the character `'` are not escaped when they are added to the list of aliases under the "yaml title alias plugin".

## How to Reproduce

Create a new vault with just this plugin installed. Enable the options:

 - escapes colons with space .... 
 - yaml title aliases

Example to reproduce issue with:

``` markdown
# It's strange
```

or 

``` markdown
---
aliases:
 - "It's strange"
---
# It's strange
```

## Expected Behavior

 - the title is escaped before being added to the list of aliases
 - if the escaped version of the title already exists in the aliases it does get added again

In both cases above the output should be 

``` markdown
---
aliases:
 - "It's strange"
---
# It's strange
```

## Actual behaviour

``` markdown
# It's strange
```
becomes
```markdown
---
aliases:
  - It's strange
---
# It's strange
```
and 
``` markdown
---
aliases:
 - "It's strange"
---
# It's strange
```
becomes
```markdown
---
aliases:
  - It's strange
  - "It's strange"
---
# It's strange
```

## Device

- [x] Desktop
- [ ] Mobile



Pull request:
Update Escape Logic and Fix Issue with Failure to Escape Titles and Aliases
Fixes #439 

Changes Made:
- Updated the UTs for yaml title and yaml title alias
- Refactored escape logic to `yaml.ts` to make sure it could be reused by other services
- Made sure that rules like yaml title and yaml title alias as well as forcing the use of escape characters all use the same logic for escaping values

Patch:
diff --git a/src/rules/escape-yaml-special-characters.ts b/src/rules/escape-yaml-special-characters.ts
index e1d9759..0589aea 100644
--- a/src/rules/escape-yaml-special-characters.ts
+++ b/src/rules/escape-yaml-special-characters.ts
@@ -1,7 +1,7 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
 import dedent from 'ts-dedent';
-import {formatYAML, isValueEscapedAlready} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML} from '../utils/yaml';
 
 class EscapeYamlSpecialCharactersOptions implements Options {
   @RuleBuilder.noSettingControl()
@@ -32,36 +32,6 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
         return text;
       }
 
-      const escapeSubstringIfNecessary = function(fullText: string, substring: string): string {
-        if (isValueEscapedAlready(substring)) {
-          return fullText;
-        }
-
-        // if there is no single quote, double quote, or colon to escape, skip this substring
-        const substringHasSingleQuote = substring.includes('\'');
-        const substringHasDoubleQuote = substring.includes('"');
-        const substringHasColonWithSpaceAfterIt = substring.includes(': ');
-        if (!substringHasSingleQuote && !substringHasDoubleQuote && !substringHasColonWithSpaceAfterIt) {
-          return fullText;
-        }
-
-        // if the substring already has a single quote and a double quote, there is nothing that can be done to escape the substring
-        if (substringHasSingleQuote && substringHasDoubleQuote) {
-          return fullText;
-        }
-
-        let newText: string;
-        if (substringHasSingleQuote) {
-          newText = fullText.replace(substring, `"${substring}"`);
-        } else if (substringHasDoubleQuote) {
-          newText = fullText.replace(substring, `'${substring}'`);
-        } else { // the line must have a colon with a space
-          newText = fullText.replace(substring, `${options.defaultEscapeCharacter}${substring}${options.defaultEscapeCharacter}`);
-        }
-
-        return newText;
-      };
-
       for (let i = 0; i < yamlLineCount; i++) {
         const line = yamlLines[i].trim();
 
@@ -99,7 +69,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
                 arrayItem = arrayItem.substring(0, arrayItem.length - 1).trimEnd();
               }
 
-              arrayItems[j] = escapeSubstringIfNecessary(arrayItems[j], arrayItem);
+              arrayItems[j] = arrayItems[j].replace(arrayItem, escapeStringIfNecessaryAndPossible(arrayItem, options.defaultEscapeCharacter));
             }
 
             yamlLines[i] = yamlLines[i].replace(value, '[' + arrayItems.join(',') + ']');
@@ -108,7 +78,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
           continue;
         }
 
-        yamlLines[i] = escapeSubstringIfNecessary(yamlLines[i], value);
+        yamlLines[i] = yamlLines[i].replace(value, escapeStringIfNecessaryAndPossible(value, options.defaultEscapeCharacter));
       }
 
       return yamlLines.join('\n');
diff --git a/src/rules/force-yaml-escape.ts b/src/rules/force-yaml-escape.ts
index 28567ee..2ed56da 100644
--- a/src/rules/force-yaml-escape.ts
+++ b/src/rules/force-yaml-escape.ts
@@ -1,7 +1,7 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {ExampleBuilder, OptionBuilderBase, TextAreaOptionBuilder} from './rule-builder';
 import dedent from 'ts-dedent';
-import {formatYAML, getYamlSectionValue, isValueEscapedAlready, setYamlSection} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML, getYamlSectionValue, isValueEscapedAlready, setYamlSection} from '../utils/yaml';
 
 class ForceYamlEscapeOptions implements Options {
   @RuleBuilder.noSettingControl()
@@ -26,7 +26,7 @@ export default class ForceYamlEscape extends RuleBuilder<ForceYamlEscapeOptions>
   apply(text: string, options: ForceYamlEscapeOptions): string {
     return formatYAML(text, (text) => {
       for (const yamlKeyToEscape of options.forceYamlEscape) {
-        const keyValue = getYamlSectionValue(text, yamlKeyToEscape);
+        let keyValue = getYamlSectionValue(text, yamlKeyToEscape);
 
         if (keyValue != null) {
           // skip yaml array values or already escaped values
@@ -34,7 +34,8 @@ export default class ForceYamlEscape extends RuleBuilder<ForceYamlEscapeOptions>
             continue;
           }
 
-          text = setYamlSection(text, yamlKeyToEscape, ` ${options.defaultEscapeCharacter}${keyValue}${options.defaultEscapeCharacter}`);
+          keyValue = escapeStringIfNecessaryAndPossible(keyValue, options.defaultEscapeCharacter, true);
+          text = setYamlSection(text, yamlKeyToEscape, ' ' + keyValue);
         }
       }
 
diff --git a/src/rules/yaml-title-alias.ts b/src/rules/yaml-title-alias.ts
index ed20aff..6247d3f 100644
--- a/src/rules/yaml-title-alias.ts
+++ b/src/rules/yaml-title-alias.ts
@@ -1,20 +1,24 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
 import dedent from 'ts-dedent';
-import {convertAliasValueToStringOrStringArray, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEY, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray, toYamlString} from '../utils/yaml';
+import {convertAliasValueToStringOrStringArray, escapeStringIfNecessaryAndPossible, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEY, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray} from '../utils/yaml';
 import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
 import {yamlRegex} from '../utils/regex';
 
 
 class YamlTitleAliasOptions implements Options {
-  @RuleBuilder.noSettingControl()
-    aliasArrayStyle?: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;
   preserveExistingAliasesSectionStyle?: boolean = true;
   keepAliasThatMatchesTheFilename?: boolean = false;
   useYamlKeyToKeepTrackOfOldFilenameOrHeading?: boolean = true;
 
+  @RuleBuilder.noSettingControl()
+    aliasArrayStyle?: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;
+
   @RuleBuilder.noSettingControl()
     fileName?: string;
+
+  @RuleBuilder.noSettingControl()
+    defaultEscapeCharacter?: string = '"';
 }
 
 @RuleBuilder.register
@@ -55,6 +59,7 @@ export default class YamlTitleAlias extends RuleBuilder<YamlTitleAliasOptions> {
 
     previousTitle = loadYAML(getYamlSectionValue(yaml, LINTER_ALIASES_HELPER_KEY));
 
+    title = escapeStringIfNecessaryAndPossible(title, options.defaultEscapeCharacter);
     const getNewAliasValue = function(originalValue: string |string[], shouldRemoveTitle: boolean): string |string[] {
       if (originalValue == null) {
         return shouldRemoveTitle ? '' : title;
@@ -97,7 +102,6 @@ export default class YamlTitleAlias extends RuleBuilder<YamlTitleAliasOptions> {
       return originalValue;
     };
 
-    title = toYamlString(title);
     if (Object.keys(parsedYaml).includes(OBSIDIAN_ALIASES_KEY)) {
       const aliasesValue = getYamlSectionValue(newYaml, OBSIDIAN_ALIASES_KEY);
       let currentAliasStyle: NormalArrayFormats | SpecialArrayFormats = NormalArrayFormats.MultiLine;
diff --git a/src/rules/yaml-title.ts b/src/rules/yaml-title.ts
index e030066..6223bdc 100644
--- a/src/rules/yaml-title.ts
+++ b/src/rules/yaml-title.ts
@@ -1,7 +1,7 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {ExampleBuilder, OptionBuilderBase, TextOptionBuilder} from './rule-builder';
 import dedent from 'ts-dedent';
-import {formatYAML, initYAML, toYamlString} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML, initYAML} from '../utils/yaml';
 import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
 import {escapeDollarSigns} from '../utils/regex';
 import {insert} from '../utils/strings';
@@ -10,6 +10,9 @@ class YamlTitleOptions implements Options {
   @RuleBuilder.noSettingControl()
     fileName: string;
 
+  @RuleBuilder.noSettingControl()
+    defaultEscapeCharacter?: string = '"';
+
   titleKey?: string = 'title';
 }
 
@@ -38,7 +41,7 @@ export default class YamlTitle extends RuleBuilder<YamlTitleOptions> {
     });
     title = title || options.fileName;
 
-    title = toYamlString(title);
+    title = escapeStringIfNecessaryAndPossible(title, options.defaultEscapeCharacter);
 
     return formatYAML(text, (text) => {
       const title_match_str = `\n${options.titleKey}.*\n`;
diff --git a/src/utils/yaml.ts b/src/utils/yaml.ts
index ce35701..fe10479 100644
--- a/src/utils/yaml.ts
+++ b/src/utils/yaml.ts
@@ -1,4 +1,4 @@
-import {load, dump} from 'js-yaml';
+import {load} from 'js-yaml';
 import {escapeDollarSigns, yamlRegex} from './regex';
 
 export const OBSIDIAN_TAG_KEY = 'tags';
@@ -29,14 +29,6 @@ export function formatYAML(text: string, func: (text: string) => string): string
   return text;
 }
 
-export function toYamlString(obj: any): string {
-  return dump(obj, {lineWidth: -1}).slice(0, -1);
-}
-
-export function toSingleLineArrayYamlString<T>(arr: T[]): string {
-  return dump(arr, {flowLevel: 0}).slice(0, -1);
-}
-
 function getYamlSectionRegExp(rawKey: string): RegExp {
   return new RegExp(`^([\\t ]*)${rawKey}:[ \\t]*(\\S.*|(?:(?:\\n *- \\S.*)|((?:\\n *- *))*|(\\n([ \\t]+[^\\n]*))*)*)\\n`, 'm');
 }
@@ -259,3 +251,39 @@ export function isValueEscapedAlready(value: string): boolean {
   return value.length > 1 && ((value.startsWith('\'') && value.endsWith('\'')) ||
     (value.startsWith('"') && value.endsWith('"')));
 }
+
+/**
+ * Escapes the provided string value if it has a colon with a space after it, a single quote, or a double quote, but not a single and double quote.
+ * @param {string} value The value to escape if possible
+ * @param {string} defaultEscapeCharacter The character escape to use around the value if a specific escape character is not needed.
+ * @param {boolean} forceEscape Whether or not to force the escaping of the value provided.
+ * @return {string} The escaped value if it is either necessary or forced and the provided value if it cannot be escaped, is escaped,
+ * or does not need escaping and the force escape is not used.
+ */
+export function escapeStringIfNecessaryAndPossible(value: string, defaultEscapeCharacter: string, forceEscape: boolean = false): string {
+  if (isValueEscapedAlready(value)) {
+    return value;
+  }
+
+  // if there is no single quote, double quote, or colon to escape, skip this substring
+  const substringHasSingleQuote = value.includes('\'');
+  const substringHasDoubleQuote = value.includes('"');
+  const substringHasColonWithSpaceAfterIt = value.includes(': ');
+  if (!substringHasSingleQuote && !substringHasDoubleQuote && !substringHasColonWithSpaceAfterIt && !forceEscape) {
+    return value;
+  }
+
+  // if the substring already has a single quote and a double quote, there is nothing that can be done to escape the substring
+  if (substringHasSingleQuote && substringHasDoubleQuote) {
+    return value;
+  }
+
+  if (substringHasSingleQuote) {
+    return `"${value}"`;
+  } else if (substringHasDoubleQuote) {
+    return `'${value}'`;
+  }
+
+  // the line must have a colon with a space
+  return `${defaultEscapeCharacter}${value}${defaultEscapeCharacter}`;
+}


Test patch:
diff --git a/__tests__/yaml-title-alias.test.ts b/__tests__/yaml-title-alias.test.ts
index 87fec00..e6f9304 100644
--- a/__tests__/yaml-title-alias.test.ts
+++ b/__tests__/yaml-title-alias.test.ts
@@ -432,17 +432,48 @@ ruleTest({
       },
     },
     {
-      testName: 'Titles with special characters are escaped',
+      testName: 'Titles with special a colon and then a space are escaped',
       before: dedent`
-        # Title with: colon, 'quote', "single quote"
+        # Title with: colon
       `,
       after: dedent`
         ---
         aliases:
-          - 'Title with: colon, ''quote'', "single quote"'
-        linter-yaml-title-alias: 'Title with: colon, ''quote'', "single quote"'
+          - 'Title with: colon'
+        linter-yaml-title-alias: 'Title with: colon'
         ---
-        # Title with: colon, 'quote', "single quote"
+        # Title with: colon
+      `,
+      options: {
+        defaultEscapeCharacter: '\'',
+      },
+    },
+    {
+      testName: 'Titles with double quote are escaped',
+      before: dedent`
+        # Title with "double quote"
+      `,
+      after: dedent`
+        ---
+        aliases:
+          - 'Title with "double quote"'
+        linter-yaml-title-alias: 'Title with "double quote"'
+        ---
+        # Title with "double quote"
+      `,
+    },
+    {
+      testName: 'Titles with single quote are escaped',
+      before: dedent`
+        # Title with 'single quote'
+      `,
+      after: dedent`
+        ---
+        aliases:
+          - "Title with 'single quote'"
+        linter-yaml-title-alias: "Title with 'single quote'"
+        ---
+        # Title with 'single quote'
       `,
     },
     {
@@ -1028,8 +1059,8 @@ ruleTest({
       after: dedent`
         ---
         aliases:
-          - '[[Heading]]'
-        linter-yaml-title-alias: '[[Heading]]'
+          - [[Heading]]
+        linter-yaml-title-alias: [[Heading]]
         ---
         [[Link1]]
 
@@ -1039,5 +1070,26 @@ ruleTest({
         aliasArrayStyle: NormalArrayFormats.MultiLine,
       },
     },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/439
+      testName: 'Make sure escaped aliases that match the H1 do not get added back',
+      before: dedent`
+        ---
+        aliases:
+          - "It's strange"
+        ---
+        # It's strange
+      `,
+      after: dedent`
+        ---
+        aliases:
+          - "It's strange"
+        ---
+        # It's strange
+      `,
+      options: {
+        aliasArrayStyle: NormalArrayFormats.MultiLine,
+        useYamlKeyToKeepTrackOfOldFilenameOrHeading: false,
+      },
+    },
   ],
 });
diff --git a/__tests__/yaml-title.test.ts b/__tests__/yaml-title.test.ts
index 1c7d63e..75320a2 100644
--- a/__tests__/yaml-title.test.ts
+++ b/__tests__/yaml-title.test.ts
@@ -24,7 +24,7 @@ ruleTest({
       `,
       after: dedent`
         ---
-        title: 'Hello: world'
+        title: "Hello: world"
         ---
         # Hello: world
       `,
@@ -36,7 +36,7 @@ ruleTest({
       `,
       after: dedent`
         ---
-        title: '''Hello world'
+        title: "'Hello world"
         ---
         # 'Hello world
       `,
@@ -74,7 +74,7 @@ ruleTest({
       `,
       after: dedent`
         ---
-        title: '[[Heading]]'
+        title: [[Heading]]
         ---
         [[Link1]]