Don't throw exception if a pattern fails to parse

src/cmd_line/commands/substitute.ts

@@ -215,7 +215,7 @@ export class SubstituteCommand extends ExCommand {
     return !this.arguments.flags.confirmEach;
   }
 
-  private getRegex(args: ISubstituteCommandArguments, vimState: VimState) {
+  private getRegex(args: ISubstituteCommandArguments, vimState: VimState): RegExp | undefined {
     let jsRegexFlags = '';
     if (configuration.gdefault || configuration.substituteGlobalFlag) {
       // the gdefault flag is on, then /g if on by default and /g negates that
@@ -238,7 +238,7 @@ export class SubstituteCommand extends ExCommand {
       // i.e. :s
       const prevSubstituteState = globalState.substituteState;
       if (
-        prevSubstituteState === undefined ||
+        prevSubstituteState?.searchPattern === undefined ||
         prevSubstituteState.searchPattern.patternString === ''
       ) {
         throw VimError.fromCode(ErrorCode.NoPreviousSubstituteRegularExpression);
@@ -266,7 +266,7 @@ export class SubstituteCommand extends ExCommand {
         vimState.currentMode
       );
     }
-    return new RegExp(args.pattern.regex.source, jsRegexFlags);
+    return args.pattern ? new RegExp(args.pattern.regex.source, jsRegexFlags) : undefined;
   }
 
   /**
@@ -396,6 +396,10 @@ export class SubstituteCommand extends ExCommand {
 
   async execute(vimState: VimState): Promise<void> {
     const regex = this.getRegex(this.arguments, vimState);
+    if (regex === undefined) {
+      return;
+    }
+
     const selection = vimState.editor.selection;
     const line = selection.start.isBefore(selection.end)
       ? selection.start.line
@@ -418,6 +422,10 @@ export class SubstituteCommand extends ExCommand {
     // TODO: Global Setting.
     // TODO: There are differencies between Vim Regex and JS Regex.
     const regex = this.getRegex(this.arguments, vimState);
+    if (regex === undefined) {
+      return;
+    }
+
     let lines = 0;
     let substitutions = 0;
     for (

src/state/searchState.ts

@@ -17,9 +17,10 @@ export class SearchState {
   ) {
     this._searchString = searchString;
 
-    const { pattern, offset } = searchStringParser({ direction, ignoreSmartcase }).tryParse(
-      searchString
-    );
+    const result = searchStringParser({ direction, ignoreSmartcase }).parse(searchString);
+    const { pattern, offset } = result.status
+      ? result.value
+      : { pattern: undefined, offset: undefined };
     this.pattern = pattern;
     this.offset = offset;
 
@@ -29,7 +30,7 @@ export class SearchState {
   }
 
   private _searchString: string;
-  public pattern: Pattern;
+  public pattern?: Pattern;
   private offset?: SearchOffset;
 
   public readonly previousMode: Mode;
@@ -40,11 +41,14 @@ export class SearchState {
   }
   public set searchString(str: string) {
     this._searchString = str;
-    const { pattern, offset } = searchStringParser({
-      direction: this.pattern.direction,
+    const result = searchStringParser({
+      direction: this.direction,
       ignoreSmartcase: this.ignoreSmartcase,
-    }).tryParse(str);
-    if (pattern.patternString !== this.pattern.patternString) {
+    }).parse(str);
+    const { pattern, offset } = result.status
+      ? result.value
+      : { pattern: undefined, offset: undefined };
+    if (pattern?.patternString !== this.pattern?.patternString) {
       this.pattern = pattern;
       this.matchRanges.clear();
     }
@@ -52,7 +56,8 @@ export class SearchState {
   }
 
   public get direction(): SearchDirection {
-    return this.pattern.direction;
+    // TODO: Defaulting to forward is wrong - I think storing the direction in the pattern is a mistake
+    return this.pattern?.direction ?? SearchDirection.Forward;
   }
 
   /**
@@ -72,7 +77,7 @@ export class SearchState {
   private readonly ignoreSmartcase: boolean;
 
   private recalculateSearchRanges(editor: TextEditor): Range[] {
-    if (this.searchString === '') {
+    if (this.searchString === '' || this.pattern === undefined) {
       return [];
     }
 
@@ -129,7 +134,7 @@ export class SearchState {
       return undefined;
     }
 
-    const effectiveDirection = (direction * this.pattern.direction) as SearchDirection;
+    const effectiveDirection = (direction * this.direction) as SearchDirection;
 
     if (effectiveDirection === SearchDirection.Forward) {
       for (const [index, range] of matchRanges.entries()) {

src/state/substituteState.ts

@@ -7,14 +7,14 @@ export class SubstituteState {
   /**
    * The last pattern searched for in the substitution
    */
-  public searchPattern: Pattern;
+  public searchPattern: Pattern | undefined;
 
   /**
    * The last replacement string in the substitution
    */
   public replaceString: string;
 
-  constructor(searchPattern: Pattern, replaceString: string) {
+  constructor(searchPattern: Pattern | undefined, replaceString: string) {
     this.searchPattern = searchPattern;
     this.replaceString = replaceString;
   }

src/vimscript/lineRange.ts

@@ -206,20 +206,22 @@ export class Address {
           if (!globalState.substituteState) {
             throw VimError.fromCode(ErrorCode.NoPreviousSubstituteRegularExpression);
           }
-          const searchState = new SearchState(
-            SearchDirection.Forward,
-            vimState.cursorStopPosition,
-            globalState.substituteState.searchPattern.patternString,
-            {},
-            vimState.currentMode
-          );
-          const match = searchState.getNextSearchMatchPosition(
+          const searchState = globalState.substituteState.searchPattern
+            ? new SearchState(
+                SearchDirection.Forward,
+                vimState.cursorStopPosition,
+                globalState.substituteState.searchPattern.patternString,
+                {},
+                vimState.currentMode
+              )
+            : undefined;
+          const match = searchState?.getNextSearchMatchPosition(
             vimState.editor,
             vimState.cursorStopPosition
           );
           if (match === undefined) {
             // TODO: throw proper errors for nowrapscan
-            throw VimError.fromCode(ErrorCode.PatternNotFound, searchState.searchString);
+            throw VimError.fromCode(ErrorCode.PatternNotFound, searchState?.searchString);
           }
           return match.pos.line;
         default:

src/vimscript/pattern.ts

@@ -38,6 +38,7 @@ export class Pattern {
   public readonly ignorecase: boolean | undefined;
 
   private static readonly MAX_SEARCH_RANGES = 1000;
+  private static readonly SPECIAL_CHARS_REGEX = /[\-\[\]{}()*+?.,\\\^$|#\s]/g;
 
   public nextMatch(document: TextDocument, fromPosition: Position): Range | undefined {
     const haystack = document.getText();
@@ -103,6 +104,16 @@ export class Pattern {
     return matchRanges.afterWrapping.concat(matchRanges.beforeWrapping);
   }
 
+  private static compileRegex(regexString: string, ignoreCase?: boolean): RegExp {
+    const flags = ignoreCase ?? configuration.ignorecase ? 'gim' : 'gm';
+    try {
+      return new RegExp(regexString, flags);
+    } catch (err) {
+      // Couldn't compile the regexp, try again with special characters escaped
+      return new RegExp(regexString.replace(Pattern.SPECIAL_CHARS_REGEX, '\\$&'), flags);
+    }
+  }
+
   public static fromLiteralString(
     input: string,
     direction: SearchDirection,
@@ -110,17 +121,9 @@ export class Pattern {
   ): Pattern {
     const patternString = input.replace(escapeRegExp(input), '\\$&');
     if (wordBoundaries) {
-      return new Pattern(
-        `\\<${patternString}\\>`,
-        direction,
-        new RegExp(`\b${patternString}\b`, configuration.ignorecase ? 'gim' : 'gm')
-      );
+      return new Pattern(`\\<${patternString}\\>`, direction, Pattern.compileRegex(patternString));
     } else {
-      return new Pattern(
-        patternString,
-        direction,
-        new RegExp(patternString, configuration.ignorecase ? 'gim' : 'gm')
-      );
+      return new Pattern(patternString, direction, Pattern.compileRegex(patternString));
     }
   }
 
@@ -177,13 +180,15 @@ export class Pattern {
         };
       })
       .map(({ patternString, caseOverride }) => {
-        const flags = Pattern.getIgnoreCase(patternString, {
+        const ignoreCase = Pattern.getIgnoreCase(patternString, {
           caseOverride,
           ignoreSmartcase: args.ignoreSmartcase ?? false,
-        })
-          ? 'gim'
-          : 'gm';
-        return new Pattern(patternString, args.direction, RegExp(patternString, flags));
+        });
+        return new Pattern(
+          patternString,
+          args.direction,
+          Pattern.compileRegex(patternString, ignoreCase)
+        );
       });
   }
 

test/mode/modeNormal.test.ts

@@ -2321,6 +2321,13 @@ suite('Mode Normal', () => {
     end: ['|x end', 'x', 'x', 'start'],
   });
 
+  newTest({
+    title: 'Search for `(`',
+    start: ['|one (two) three'],
+    keysPressed: '/(\n',
+    end: ['one |(two) three'],
+  });
+
   /**
    * The escaped `/` and `?` the next tests are necessary because otherwise they denote a search offset.
    */