Repository: platers/obsidian-linter (ts)

Issue:
Bug: `YAML Title Alias` rule fails if the note name starts with exclamation mark `!`
## Describe the Bug

`YAML Title Alias` rule fails if the note name starts with exclamation mark `!`

## How to Reproduce

1. Create a note named `!Note.md`
2. Apply `YAML Title Alias` rule to it

## Expected Behavior

No errors

## Actual Behavior

```
logger.ts:21 [Obsidian Linter] Lint File Error in File '!Note.md':
Error "YAML Title Alias" encountered an error in the yaml:  unknown tag !<!Note> (2:1)

 1 | title: !Note
 2 | 
-----^
YAMLException: unknown tag !<!Note> (2:1)

 1 | title: !Note
 2 | 
-----^
    at generateError (plugin:obsidian-linter:2695:10)
    at throwError (plugin:obsidian-linter:2698:9)
    at composeNode (plugin:obsidian-linter:3603:7)
    at readBlockMapping (plugin:obsidian-linter:3353:11)
    at composeNode (plugin:obsidian-linter:3546:78)
    at readDocument (plugin:obsidian-linter:3678:3)
    at loadDocuments (plugin:obsidian-linter:3720:5)
    at load$1 (plugin:obsidian-linter:3738:19)
    at loadYAML (plugin:obsidian-linter:4602:23)
    at YamlTitleAlias.apply (plugin:obsidian-linter:53076:24)
```

Pull request:
Perform escaping validation
Fixes #614 

I had to add a hack to skip the new logic for `Escape YAML Special Characters` rule, because the new approach would not work fine with abovementioned rule.

The idea was to ensure that in difficult scenarios use `js-yaml` serialization techniques.

So if we have a title `- title` we see that it deserializes as an array instead of string, so we conclude that we need to enclose the title in quotes `"- title"`.

However such logic is fine when we are sure that we were escaping only the string.

But `Escape YAML Special Characters` is executed when the entire YAML object is processed, so each line is not meant to be a string, therefore abovementioned logic would not make sense.

I'd prefer `Escape YAML Special Characters` rule to be refactored to take advantage of the improvements I've made, but I think this would require rewriting that rule entirely, ideally treating the YAML as an AST, not by manual parsing line-by-line.

I also noticed that `Escape YAML Special Characters` might produce an invalid YAML, which in my opinion, we should avoid by all means.

Patch:
diff --git a/src/rules.ts b/src/rules.ts
index f07fec1..48cafc0 100644
--- a/src/rules.ts
+++ b/src/rules.ts
@@ -1,6 +1,7 @@
 import {
   getYamlSectionValue,
   loadYAML,
+  QuoteCharacter,
 } from './utils/yaml';
 import {
   Option,
@@ -22,7 +23,7 @@ export type CommonStyles = {
   aliasArrayStyle: NormalArrayFormats | SpecialArrayFormats;
   tagArrayStyle: TagSpecificArrayFormats | NormalArrayFormats | SpecialArrayFormats;
   minimumNumberOfDollarSignsToBeAMathBlock: number;
-  escapeCharacter: string;
+  escapeCharacter: QuoteCharacter;
   removeUnnecessaryEscapeCharsForMultiLineArrays: boolean;
 }
 
diff --git a/src/rules/escape-yaml-special-characters.ts b/src/rules/escape-yaml-special-characters.ts
index 9ccc498..004b8a3 100644
--- a/src/rules/escape-yaml-special-characters.ts
+++ b/src/rules/escape-yaml-special-characters.ts
@@ -1,11 +1,11 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
 import dedent from 'ts-dedent';
-import {escapeStringIfNecessaryAndPossible, formatYAML} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML, QuoteCharacter} from '../utils/yaml';
 
 class EscapeYamlSpecialCharactersOptions implements Options {
   @RuleBuilder.noSettingControl()
-    defaultEscapeCharacter?: string = '"';
+    defaultEscapeCharacter?: QuoteCharacter = '"';
   tryToEscapeSingleLineArrays?: boolean = false;
 }
 
@@ -84,7 +84,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
                 arrayItem = arrayItem.substring(0, arrayItem.length - 1).trimEnd();
               }
 
-              arrayItems[j] = arrayItems[j].replace(arrayItem, escapeStringIfNecessaryAndPossible(arrayItem, options.defaultEscapeCharacter));
+              arrayItems[j] = arrayItems[j].replace(arrayItem, escapeStringIfNecessaryAndPossible(arrayItem, options.defaultEscapeCharacter, false, true));
             }
 
             yamlLines[i] = yamlLines[i].replace(value, '[' + arrayItems.join(',') + ']');
@@ -93,7 +93,7 @@ export default class EscapeYamlSpecialCharacters extends RuleBuilder<EscapeYamlS
           continue;
         }
 
-        yamlLines[i] = yamlLines[i].replace(value, escapeStringIfNecessaryAndPossible(value, options.defaultEscapeCharacter));
+        yamlLines[i] = yamlLines[i].replace(value, escapeStringIfNecessaryAndPossible(value, options.defaultEscapeCharacter, false, true));
       }
 
       return yamlLines.join('\n');
diff --git a/src/rules/force-yaml-escape.ts b/src/rules/force-yaml-escape.ts
index ea3b445..cb4a232 100644
--- a/src/rules/force-yaml-escape.ts
+++ b/src/rules/force-yaml-escape.ts
@@ -1,11 +1,11 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {ExampleBuilder, OptionBuilderBase, TextAreaOptionBuilder} from './rule-builder';
 import dedent from 'ts-dedent';
-import {escapeStringIfNecessaryAndPossible, formatYAML, getYamlSectionValue, isValueEscapedAlready, setYamlSection} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML, getYamlSectionValue, isValueEscapedAlready, QuoteCharacter, setYamlSection} from '../utils/yaml';
 
 class ForceYamlEscapeOptions implements Options {
   @RuleBuilder.noSettingControl()
-    defaultEscapeCharacter?: string = '"';
+    defaultEscapeCharacter?: QuoteCharacter = '"';
   forceYamlEscape?: string[] = [];
 }
 
diff --git a/src/rules/yaml-title-alias.ts b/src/rules/yaml-title-alias.ts
index 20e3304..d5480c5 100644
--- a/src/rules/yaml-title-alias.ts
+++ b/src/rules/yaml-title-alias.ts
@@ -1,7 +1,7 @@
 import {Options, RuleType} from '../rules';
 import RuleBuilder, {BooleanOptionBuilder, ExampleBuilder, OptionBuilderBase} from './rule-builder';
 import dedent from 'ts-dedent';
-import {convertAliasValueToStringOrStringArray, escapeStringIfNecessaryAndPossible, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEYS, OBSIDIAN_ALIAS_KEY_PLURAL, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray} from '../utils/yaml';
+import {convertAliasValueToStringOrStringArray, escapeStringIfNecessaryAndPossible, formatYamlArrayValue, getYamlSectionValue, initYAML, LINTER_ALIASES_HELPER_KEY, loadYAML, NormalArrayFormats, OBSIDIAN_ALIASES_KEYS, OBSIDIAN_ALIAS_KEY_PLURAL, QuoteCharacter, removeYamlSection, setYamlSection, SpecialArrayFormats, splitValueIfSingleOrMultilineArray} from '../utils/yaml';
 import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
 import {getFirstHeaderOneText, yamlRegex} from '../utils/regex';
 
@@ -17,7 +17,7 @@ class YamlTitleAliasOptions implements Options {
     fileName?: string;
 
   @RuleBuilder.noSettingControl()
-    defaultEscapeCharacter?: string = '"';
+    defaultEscapeCharacter?: QuoteCharacter = '"';
 
   @RuleBuilder.noSettingControl()
     removeUnnecessaryEscapeCharsForMultiLineArrays?: boolean = false;
diff --git a/src/rules/yaml-title.ts b/src/rules/yaml-title.ts
index 7ec33b8..99712ce 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 {escapeStringIfNecessaryAndPossible, formatYAML, initYAML} from '../utils/yaml';
+import {escapeStringIfNecessaryAndPossible, formatYAML, initYAML, QuoteCharacter} from '../utils/yaml';
 import {ignoreListOfTypes, IgnoreTypes} from '../utils/ignore-types';
 import {escapeDollarSigns, getFirstHeaderOneText} from '../utils/regex';
 import {insert} from '../utils/strings';
@@ -11,7 +11,7 @@ class YamlTitleOptions implements Options {
     fileName: string;
 
   @RuleBuilder.noSettingControl()
-    defaultEscapeCharacter?: string = '"';
+    defaultEscapeCharacter?: QuoteCharacter = '"';
 
   titleKey?: string = 'title';
 }
diff --git a/src/ui/linter-components/tab-components/general-tab.ts b/src/ui/linter-components/tab-components/general-tab.ts
index 738e94c..eeddea3 100644
--- a/src/ui/linter-components/tab-components/general-tab.ts
+++ b/src/ui/linter-components/tab-components/general-tab.ts
@@ -3,7 +3,7 @@ import {Tab} from './tab';
 import {Setting} from 'obsidian';
 import {moment} from 'obsidian';
 import {parseTextToHTMLWithoutOuterParagraph} from 'src/ui/helpers';
-import {NormalArrayFormats, SpecialArrayFormats, TagSpecificArrayFormats} from 'src/utils/yaml';
+import {NormalArrayFormats, QuoteCharacter, SpecialArrayFormats, TagSpecificArrayFormats} from 'src/utils/yaml';
 import {getTextInLanguage} from 'src/lang/helpers';
 
 export class GeneralTab extends Tab {
@@ -145,7 +145,7 @@ export class GeneralTab extends Tab {
 
     this.addSettingSearchInfo(tempDiv, settingName, settingDesc);
 
-    const escapeCharRecords = ['"', '\''];
+    const escapeCharRecords: QuoteCharacter[] = ['"', '\''];
 
     tempDiv = this.contentEl.createDiv();
     settingName = getTextInLanguage('default-escape-character-name');
@@ -159,7 +159,7 @@ export class GeneralTab extends Tab {
           });
           dropdown.setValue(this.plugin.settings.commonStyles.escapeCharacter);
           dropdown.onChange(async (value) => {
-            this.plugin.settings.commonStyles.escapeCharacter = value;
+            this.plugin.settings.commonStyles.escapeCharacter = value as QuoteCharacter;
             await this.plugin.saveSettings();
           });
         });
diff --git a/src/utils/yaml.ts b/src/utils/yaml.ts
index 85f7a15..65fd956 100644
--- a/src/utils/yaml.ts
+++ b/src/utils/yaml.ts
@@ -1,4 +1,4 @@
-import {load} from 'js-yaml';
+import {load, dump} from 'js-yaml';
 import {getTextInLanguage} from '../lang/helpers';
 import {escapeDollarSigns, yamlRegex} from './regex';
 
@@ -94,6 +94,8 @@ export enum NormalArrayFormats {
   MultiLine = 'multi-line',
 }
 
+export type QuoteCharacter = '\'' | '"';
+
 /**
  * Formats the yaml array value passed in with the specified format.
  * @param {string | string[]} value The value(s) that will be used as the parts of the array that is assumed to already be broken down into the appropriate format to be put in the array.
@@ -102,7 +104,7 @@ export enum NormalArrayFormats {
  * @param {boolean} removeEscapeCharactersIfPossibleWhenGoingToMultiLine Whether or not to remove no longer needed escape values when converting to a multi-line format.
  * @return {string} The formatted array in the specified yaml/obsidian yaml format.
  */
-export function formatYamlArrayValue(value: string | string[], format: NormalArrayFormats | SpecialArrayFormats | TagSpecificArrayFormats, defaultEscapeCharacter: string, removeEscapeCharactersIfPossibleWhenGoingToMultiLine: boolean): string {
+export function formatYamlArrayValue(value: string | string[], format: NormalArrayFormats | SpecialArrayFormats | TagSpecificArrayFormats, defaultEscapeCharacter: QuoteCharacter, removeEscapeCharactersIfPossibleWhenGoingToMultiLine: boolean): string {
   if (typeof value === 'string') {
     value = [value];
   }
@@ -325,10 +327,45 @@ export function isValueEscapedAlready(value: string): boolean {
  * @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.
+ * @param {boolean} skipValidation Whether or not to ensure that the result string could be unescaped back to the value.
  * @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 {
+export function escapeStringIfNecessaryAndPossible(value: string, defaultEscapeCharacter: QuoteCharacter, forceEscape: boolean = false, skipValidation: boolean = false): string {
+  const basicEscape = basicEscapeString(value, defaultEscapeCharacter, forceEscape);
+  if (skipValidation) {
+    return basicEscape;
+  }
+
+  try {
+    const unescaped = load(basicEscape) as string;
+    if (unescaped === value) {
+      return basicEscape;
+    }
+  } catch {
+    // invalid YAML
+  }
+
+  const escapeWithDefaultCharacter = dump(value, {
+    lineWidth: -1,
+    quotingType: defaultEscapeCharacter,
+    forceQuotes: forceEscape,
+  }).slice(0, -1);
+
+  const escapeWithOtherCharacter = dump(value, {
+    lineWidth: -1,
+    quotingType: defaultEscapeCharacter == '"' ? '\'' : '"',
+    forceQuotes: forceEscape,
+  }).slice(0, -1);
+
+  if (escapeWithOtherCharacter === value || escapeWithOtherCharacter.length < escapeWithDefaultCharacter.length) {
+    return escapeWithOtherCharacter;
+  }
+
+  return escapeWithDefaultCharacter;
+}
+
+function basicEscapeString(value: string, defaultEscapeCharacter: QuoteCharacter, forceEscape: boolean = false): string {
   if (isValueEscapedAlready(value)) {
     return value;
   }


Test patch:
diff --git a/__tests__/escape-yaml-special-characters.test.ts b/__tests__/escape-yaml-special-characters.test.ts
index bdd3057..4b43e7a 100644
--- a/__tests__/escape-yaml-special-characters.test.ts
+++ b/__tests__/escape-yaml-special-characters.test.ts
@@ -1,9 +1,9 @@
 import dedent from 'ts-dedent';
 import {ruleTest} from './common';
-import EscapeYamlSpecialCharactersOptions from '../src/rules/escape-yaml-special-characters';
+import EscapeYamlSpecialCharacters from '../src/rules/escape-yaml-special-characters';
 
 ruleTest({
-  RuleBuilderClass: EscapeYamlSpecialCharactersOptions,
+  RuleBuilderClass: EscapeYamlSpecialCharacters,
   testCases: [
     { // accounts for https://github.com/platers/obsidian-linter/issues/467
       testName: 'Make sure that dictionaries in a multiline array do not accidentally get escaped',
diff --git a/__tests__/yaml-title.test.ts b/__tests__/yaml-title.test.ts
index 54eee2e..a7b0a65 100644
--- a/__tests__/yaml-title.test.ts
+++ b/__tests__/yaml-title.test.ts
@@ -18,7 +18,7 @@ ruleTest({
       `,
     },
     {
-      testName: 'Escapes title if it contains colon',
+      testName: 'Escapes title if it contains colon followed by space',
       before: dedent`
         # Hello: world
       `,
@@ -153,5 +153,161 @@ ruleTest({
         # [[Broken Linter|Broken]] Linter
       `,
     },
+    {
+      testName: 'Escapes title if it starts with exclamation mark',
+      before: dedent`
+        # !title
+      `,
+      after: dedent`
+        ---
+        title: "!title"
+        ---
+        # !title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with at sign',
+      before: dedent`
+        # @title
+      `,
+      after: dedent`
+        ---
+        title: "@title"
+        ---
+        # @title
+      `,
+    },
+    {
+      testName: 'Escapes title if it contains hash',
+      before: dedent`
+        # prefix #title
+      `,
+      after: dedent`
+        ---
+        title: "prefix #title"
+        ---
+        # prefix #title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with percent',
+      before: dedent`
+        # %title
+      `,
+      after: dedent`
+        ---
+        title: "%title"
+        ---
+        # %title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with ampersand',
+      before: dedent`
+        # &title
+      `,
+      after: dedent`
+        ---
+        title: "&title"
+        ---
+        # &title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with asterisk',
+      before: dedent`
+        # *title
+      `,
+      after: dedent`
+        ---
+        title: "*title"
+        ---
+        # *title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with hyphen followed by space',
+      before: dedent`
+        # - title
+      `,
+      after: dedent`
+        ---
+        title: "- title"
+        ---
+        # - title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with left square bracket',
+      before: dedent`
+        # [title
+      `,
+      after: dedent`
+        ---
+        title: "[title"
+        ---
+        # [title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with right square bracket',
+      before: dedent`
+        # ]title
+      `,
+      after: dedent`
+        ---
+        title: "]title"
+        ---
+        # ]title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with left curly bracket',
+      before: dedent`
+        # {title
+      `,
+      after: dedent`
+        ---
+        title: "{title"
+        ---
+        # {title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with right curly bracket',
+      before: dedent`
+        # }title
+      `,
+      after: dedent`
+        ---
+        title: "}title"
+        ---
+        # }title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with vertical bar',
+      before: dedent`
+        # |title
+      `,
+      after: dedent`
+        ---
+        title: "|title"
+        ---
+        # |title
+      `,
+    },
+    {
+      testName: 'Escapes title if it starts with greater-than sign',
+      before: dedent`
+        # >title
+      `,
+      after: dedent`
+        ---
+        title: ">title"
+        ---
+        # >title
+      `,
+    },
   ],
 });