Repository: platers/obsidian-linter (ts)

Issue:
Bug: Re-Index footnotes also moves footnotes to the bottom
- [x] I have verified that I am on the latest version of the Linter

## Describe the Bug

*Re-Index Footnotes* moves footnotes to the bottom, even when *Move Footnotes to the bottom* is unticked.

## How to Reproduce

Steps to reproduce the behavior:

1. In Linter settings, make sure *Re-index Footnotes* is turned on and *Move Footnotes to the bottom* is turned off.
2. Make a new document with the following text (without blockquotes):

```markdown
foo [^1]

[^1]: bar

foobar
```

3. Lint manually or via auto-lint on save.

## Expected Behavior

Expected behavior is that nothing should happen.

## Device

- [x] Desktop
- [x] Mobile (iOS)


Pull request:
Rework Footnote Logic
Fixes #902 

There was an issue where re-index footnotes was moving them to the end of the document. This was caused by trying to sort the references by their location in the document which is handled by the rule for moving footnotes to the end of the document. Thus that logic was removed. Re-index footnotes was then updated to make sure it properly handled removing duplicate footnote definitions.

Changes Made:
- Moved the test for sorting index values to move footnotes to end of file where it belongs since re-index should not be moving footnotes to the end of the file and move footnotes already does the sorting
- Added test for making sure that footnote definitions are left where they are in the file
- Duplicate footnote definitions are removed where they are at
  - For inline definitions that means they are removed in place
  - Definitions on their own line are removed with their line
-  Added some expect error info for some typescript warnings around mdast and fixed a function call that was not properly setup
- Cleaned up and condensed the footnote related logic to make it simpler and work more like what is expected

Patch:
diff --git a/docs/docs/settings/footnote-rules.md b/docs/docs/settings/footnote-rules.md
index 8ccf204..05fe38a 100644
--- a/docs/docs/settings/footnote-rules.md
+++ b/docs/docs/settings/footnote-rules.md
@@ -180,7 +180,6 @@ After:
 
 `````` markdown
 bla[^1], bla[^1], bla[^2]
-
 [^1]: bla
 [^2]: bla
 ``````
diff --git a/src/rules/re-index-footnotes.ts b/src/rules/re-index-footnotes.ts
index fed0573..07dc0a7 100644
--- a/src/rules/re-index-footnotes.ts
+++ b/src/rules/re-index-footnotes.ts
@@ -83,7 +83,6 @@ export default class ReIndexFootnotes extends RuleBuilder<ReIndexFootnotesOption
         `,
         after: dedent`
           bla[^1], bla[^1], bla[^2]
-          ${''}
           [^1]: bla
           [^2]: bla
         `,
diff --git a/src/utils/mdast.ts b/src/utils/mdast.ts
index 6ab9e24..fa48f94 100644
--- a/src/utils/mdast.ts
+++ b/src/utils/mdast.ts
@@ -59,8 +59,9 @@ function parseTextToAST(text: string): Root {
     return LRU.get(textHash) as Root;
   }
 
+  // @ts-expect-error for some reason an overload is missing
   const ast = fromMarkdown(text, {
-    extensions: [combineExtensions([gfmFootnote(), gfmTaskListItem]), math()],
+    extensions: [combineExtensions([gfmFootnote(), gfmTaskListItem()]), math()],
     mdastExtensions: [[
       gfmFootnoteFromMarkdown(),
       gfmTaskListItemFromMarkdown,
@@ -173,7 +174,21 @@ export function moveFootnotesToEnd(text: string): string {
     footnoteKeyToFootnoteKeyInfo.set(footnoteReference, keyInfo);
   };
 
-  text = removeFootnotesAndDoAnActionThem(positions, text, footnotes, getAllReferencePositionsForFootnote);
+  for (const position of positions) {
+    const footnote = text.substring(position.start.offset, position.end.offset);
+    footnotes.push(footnote);
+    // Remove the newline after the footnote if it exists
+    if (position.end.offset < text.length && text[position.end.offset] === '\n') {
+      text = text.substring(0, position.end.offset) + text.substring(position.end.offset + 1);
+    }
+    // Remove the newline after the footnote if it exists
+    if (position.end.offset < text.length && text[position.end.offset] === '\n') {
+      text = text.substring(0, position.end.offset) + text.substring(position.end.offset + 1);
+    }
+    text = text.substring(0, position.start.offset) + text.substring(position.end.offset);
+
+    getAllReferencePositionsForFootnote(text, footnote, position.start.offset);
+  }
 
   for (const footnoteData of footnoteKeyToFootnoteKeyInfo) {
     const keyInfo = footnoteData[1];
@@ -207,67 +222,60 @@ export function moveFootnotesToEnd(text: string): string {
 }
 
 /**
- * Re-indexes the footnotes in the document making sure that they increase in number from 1 on up
- * and reorders the footnotes themselves as well.
+ * Re-indexes the footnotes in the document making sure that they increase in number from 1 on up.
  * @param {string} text - The text to re-index the footnotes in.
  * @return {string} The text with footnotes re-indexed.
  */
 export function reIndexFootnotes(text: string): string {
   const positions: Position[] = getPositions(MDAstTypes.Footnote, text);
-  let footnotes: string[] = [];
+  const footnotes: string[] = [];
+
+  type keyInfo = {
+    key: string,
+    position: number,
+  }
 
-  const mapOfFootnoteToFirstFootnoteReferenceIndex = new Map<string, number>();
   const footnoteToFootnoteKey = new Map<string, string>();
   const oldKeyToNewKey = new Map<string, string>();
-  let footnoteReferenceLocationInfo: {key: string, position: number}[] = [];
+  const footnoteReferenceLocationInfo: keyInfo[] = [];
   const footnoteKeys = new Set<string>();
+  const duplicateFootnotesToReplace: string[] = [];
 
-  const getFirstReferenceToFootnote = function(text: string, footnote: string, startOfFootnoteReferenceSearch: number): number {
+  const getAllFootnoteReferences = function(text: string, footnote: string, startOfFootnoteReferenceSearch: number): void {
     const footnoteReference = footnote.match(/\[\^.*?\]/)[0];
     footnoteToFootnoteKey.set(footnote, footnoteReference);
 
     const footnoteKeyAlreadyUsed = footnoteKeys.has(footnoteReference);
-    if (footnoteKeyAlreadyUsed && mapOfFootnoteToFirstFootnoteReferenceIndex.has(footnote)) {
-      return mapOfFootnoteToFirstFootnoteReferenceIndex.get(footnote);
+    if (footnoteKeyAlreadyUsed && footnotes.includes(footnote)) {
+      duplicateFootnotesToReplace.unshift(footnote);
+
+      return;
     } else if (footnoteKeyAlreadyUsed) {
       throw new Error(getTextInLanguage('logs.too-many-footnotes-error-message').replace('{FOOTNOTE_KEY}', footnoteReference));
     }
 
     let footnoteReferenceLocation: number;
-    let firstFootnoteReferenceIndex: number = -1;
     do {
       footnoteReferenceLocation = text.lastIndexOf(footnoteReference, startOfFootnoteReferenceSearch);
       if (footnoteReferenceLocation === -1) {
         continue;
       }
 
-      footnoteReferenceLocationInfo.push({key: footnoteReference, position: footnoteReferenceLocation});
-      firstFootnoteReferenceIndex = footnoteReferenceLocation;
+      if (footnoteReferenceLocation + footnote.length > text.length || text.substring(footnoteReferenceLocation, footnoteReferenceLocation + footnote.length) !== footnote) {
+        footnoteReferenceLocationInfo.push({key: footnoteReference, position: footnoteReferenceLocation});
+      }
+
       startOfFootnoteReferenceSearch = footnoteReferenceLocation - 1;
     } while (footnoteReferenceLocation > 0);
 
     footnoteKeys.add(footnoteReference);
-
-    return firstFootnoteReferenceIndex;
   };
 
-  text = removeFootnotesAndDoAnActionThem(positions, text, footnotes, (text: string, footnote: string, startOfFootnoteReferenceSearch: number) => {
-    mapOfFootnoteToFirstFootnoteReferenceIndex.set(footnote, getFirstReferenceToFootnote(text, footnote, startOfFootnoteReferenceSearch));
-  });
-
-  // Sort the footnotes into the order of their references in the text
-  footnotes = footnotes.sort((f1: string, f2: string) => {
-    return mapOfFootnoteToFirstFootnoteReferenceIndex.get(f1) - mapOfFootnoteToFirstFootnoteReferenceIndex.get(f2);
-  });
-
-  // Sort the footnote references from last to first to prevent issues when replacing the footnote references down the road
-  footnoteReferenceLocationInfo = footnoteReferenceLocationInfo.sort((f1: {key: string, position: number}, f2: {key: string, position: number}) => {
-    return f2.position - f1.position;
-  });
+  for (const position of positions) {
+    const footnote = text.substring(position.start.offset, position.end.offset);
+    footnotes.unshift(footnote);
 
-  // Add the footnotes to the end of the document
-  if (footnotes.length > 0) {
-    text = text.trimEnd() + '\n';
+    getAllFootnoteReferences(text, footnote, position.start.offset);
   }
 
   let footnoteIndex = 1;
@@ -281,34 +289,33 @@ export function reIndexFootnotes(text: string): string {
     const footnoteKey = footnoteToFootnoteKey.get(footnote);
     const newFootnoteKey = `[^${footnoteIndex++}]`;
     oldKeyToNewKey.set(footnoteKey, newFootnoteKey);
-
-    text += '\n' + footnote.replace(footnoteKey, newFootnoteKey);
   }
 
+  footnoteReferenceLocationInfo.sort((pos1: keyInfo, pos2: keyInfo) => {
+    return pos2.position - pos1.position;
+  });
+
+  // replace the values that are tied to existing positions from last to first first since replace works even if positions change
   for (const footnoteReference of footnoteReferenceLocationInfo) {
     const newFootnoteKey = oldKeyToNewKey.get(footnoteReference.key);
 
     text = replaceAt(text, footnoteReference.key, newFootnoteKey, footnoteReference.position);
   }
 
-  return text;
-}
+  for (const footnote of footnotesAdded) {
+    const footnoteKey = footnoteToFootnoteKey.get(footnote);
+    const newFootnoteKey = oldKeyToNewKey.get(footnoteKey);
 
-function removeFootnotesAndDoAnActionThem(positions: Position[], text: string, footnotes: string[], action: (text: string, footnote: string, startOfFootnoteReferenceSearch: number) => void) {
-  for (const position of positions) {
-    const footnote = text.substring(position.start.offset, position.end.offset);
-    footnotes.push(footnote);
-    // Remove the newline after the footnote if it exists
-    if (position.end.offset < text.length && text[position.end.offset] === '\n') {
-      text = text.substring(0, position.end.offset) + text.substring(position.end.offset + 1);
-    }
-    // Remove the newline after the footnote if it exists
-    if (position.end.offset < text.length && text[position.end.offset] === '\n') {
-      text = text.substring(0, position.end.offset) + text.substring(position.end.offset + 1);
+    text = text.replace(footnote, footnote.replace(footnoteKey, newFootnoteKey));
+  }
+
+  for (const duplicateFootnoteDefinition of duplicateFootnotesToReplace) {
+    let newText = text.replace(`\n${duplicateFootnoteDefinition}\n`, '\n');
+    if (text === newText) {
+      newText = text.replace(duplicateFootnoteDefinition, '');
     }
-    text = text.substring(0, position.start.offset) + text.substring(position.end.offset);
 
-    action(text, footnote, position.start.offset);
+    text = newText;
   }
 
   return text;
diff --git a/src/utils/regex.ts b/src/utils/regex.ts
index d81ebdc..f8ce36b 100644
--- a/src/utils/regex.ts
+++ b/src/utils/regex.ts
@@ -45,7 +45,7 @@ export const nonBlockquoteChecklistRegex = new RegExp(`^\\s*- ${checklistBoxIndi
 
 export const startsWithListMarkerRegex = new RegExp(`^\\s*(-|\\*|\\+|\\d+[.)]|- (${checklistBoxIndicator}))`, 'm');
 
-export const footnoteDefinitionIndicatorAtStartOfLine = /^(\[\^\w+\]) ?([,.;!:?])/gm;
+export const footnoteDefinitionIndicatorAtStartOfLine = /^(\[\^[^\]]*\]) ?([,.;!:?])/gm;
 export const calloutRegex = /^(>\s*)+\[![^\s]*\]/m;
 
 export const unicodeLetterRegex = RegExp(/\p{L}/, 'u');


Test patch:
diff --git a/__tests__/move-footnotes-to-the-bottom.test.ts b/__tests__/move-footnotes-to-the-bottom.test.ts
index 79e5e99..2938d7c 100644
--- a/__tests__/move-footnotes-to-the-bottom.test.ts
+++ b/__tests__/move-footnotes-to-the-bottom.test.ts
@@ -204,5 +204,75 @@ ruleTest({
         [^2]: b
       `,
     },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/359
+      testName: 'Moving footnotes to the end of the file sorts footnote definitions by first reference in document',
+      before: dedent`
+        aaaaaa[^1]
+        cccccc[^3]
+        ddddd[^4]
+        bbbbb[^2]
+        fffffff[^5]
+        ${''}
+        [^1]: a-footnote
+        [^2]: b-footnote
+        [^3]: c-footnote
+        [^4]: d-footnote
+        [^5]: f-footnote
+      `,
+      after: dedent`
+        aaaaaa[^1]
+        cccccc[^3]
+        ddddd[^4]
+        bbbbb[^2]
+        fffffff[^5]
+        ${''}
+        [^1]: a-footnote
+        [^3]: c-footnote
+        [^4]: d-footnote
+        [^2]: b-footnote
+        [^5]: f-footnote
+      `,
+    },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/902
+      testName: 'Moving footnotes to the end of the file should make sure that there are no empty lines between footnotes',
+      before: dedent`
+        AAA[^1]
+        ${''}
+        BBB[^2]
+        ${''}
+        Something[^foo-bar]
+        ${''}
+        CCC[^hello-world]
+        ${''}
+        DDD[^3]
+        ${''}
+        [^foo-bar]: Some text.
+        [^1]: A
+        [^2]: B
+        [^hello-world]: C
+        [^3]: D
+        ${''}
+        More text.
+      `,
+      after: dedent`
+        AAA[^1]
+        ${''}
+        BBB[^2]
+        ${''}
+        Something[^foo-bar]
+        ${''}
+        CCC[^hello-world]
+        ${''}
+        DDD[^3]
+        ${''}
+        More text.
+        ${''}
+        [^1]: A
+        [^2]: B
+        [^foo-bar]: Some text.
+        [^hello-world]: C
+        [^3]: D
+      `,
+    },
   ],
 });
diff --git a/__tests__/paragraph-blank-lines.test.ts b/__tests__/paragraph-blank-lines.test.ts
index c692302..9636650 100644
--- a/__tests__/paragraph-blank-lines.test.ts
+++ b/__tests__/paragraph-blank-lines.test.ts
@@ -349,5 +349,46 @@ ruleTest({
         [^3]: [github.com](https://github.com)
       `,
     },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/902
+      testName: 'Make sure that if a footnote definition has a dash in it it is properly treated like a footnote definition and the paragraph is ignored',
+      before: dedent`
+        AAA[^2]
+        ${''}
+        BBB[^3]
+        ${''}
+        Something[^test-foot]
+        ${''}
+        CCC[^4]
+        ${''}
+        DDD[^5]
+        ${''}
+        More text.
+        ${''}
+        [^test-foot]: Some text.
+        [^2]: A
+        [^3]: B
+        [^4]: C
+        [^5]: D
+      `,
+      after: dedent`
+        AAA[^2]
+        ${''}
+        BBB[^3]
+        ${''}
+        Something[^test-foot]
+        ${''}
+        CCC[^4]
+        ${''}
+        DDD[^5]
+        ${''}
+        More text.
+        ${''}
+        [^test-foot]: Some text.
+        [^2]: A
+        [^3]: B
+        [^4]: C
+        [^5]: D
+      `,
+    },
   ],
 });
diff --git a/__tests__/re-index-footnotes.test.ts b/__tests__/re-index-footnotes.test.ts
index 6d458b5..37b4eb6 100644
--- a/__tests__/re-index-footnotes.test.ts
+++ b/__tests__/re-index-footnotes.test.ts
@@ -14,33 +14,83 @@ ruleTest({
         \`h[^ae]llo\`
       `,
     },
-    { // accounts for https://github.com/platers/obsidian-linter/issues/359
-      testName: 'Out of order references are sorted into their proper our based on the order in which they first appear in the file',
+    { // accounts for https://github.com/platers/obsidian-linter/issues/902
+      testName: 'Re-indexing footnotes does not move the footnote references',
       before: dedent`
-        aaaaaa[^1]
-        cccccc[^3]
-        ddddd[^4]
-        bbbbb[^2]
-        fffffff[^5]
-        ${''}
-        [^1]: a-footnote
-        [^2]: b-footnote
-        [^3]: c-footnote
-        [^4]: d-footnote
-        [^5]: f-footnote
+        foo [^1]
+        ${''}
+        [^1]: bar
+        ${''}
+        foobar
+      `,
+      after: dedent`
+        foo [^1]
+        ${''}
+        [^1]: bar
+        ${''}
+        foobar
+      `,
+    },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/902
+      testName: 'Re-indexing footnotes with `-` in their name should work',
+      before: dedent`
+        text [^numero]
+        ${''}
+        [^numero]: value
+        ${''}
+        more text [^num-1]
+        [^num-1]: number 1
+        more text2
+      `,
+      after: dedent`
+        text [^1]
+        ${''}
+        [^1]: value
+        ${''}
+        more text [^2]
+        [^2]: number 1
+        more text2
+      `,
+    },
+    { // accounts for https://github.com/platers/obsidian-linter/issues/902
+      testName: 'Re-indexing footnotes with number and non-numbered names are properly handled',
+      before: dedent`
+        AAA[^1]
+        ${''}
+        BBB[^2]
+        ${''}
+        Something[^foo-bar]
+        ${''}
+        CCC[^hello-world]
+        ${''}
+        DDD[^3]
+        ${''}
+        [^foo-bar]: Some text.
+        [^1]: A
+        [^2]: B
+        [^hello-world]: C
+        [^3]: D
+        ${''}
+        More text.
       `,
       after: dedent`
-        aaaaaa[^1]
-        cccccc[^2]
-        ddddd[^3]
-        bbbbb[^4]
-        fffffff[^5]
-        ${''}
-        [^1]: a-footnote
-        [^2]: c-footnote
-        [^3]: d-footnote
-        [^4]: b-footnote
-        [^5]: f-footnote
+        AAA[^2]
+        ${''}
+        BBB[^3]
+        ${''}
+        Something[^1]
+        ${''}
+        CCC[^4]
+        ${''}
+        DDD[^5]
+        ${''}
+        [^1]: Some text.
+        [^2]: A
+        [^3]: B
+        [^4]: C
+        [^5]: D
+        ${''}
+        More text.
       `,
     },
   ],