Tweak handling of missing } (#1985)

* remove m4 filter shortcut * remove tests too * Update usage of closeCurly * fix another test
malloydata · Dec 31, 2024 · b3338ca · b3338ca
1 parent be9f2bb
commit b3338ca
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 58 deletions.
diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4
@@ -27,13 +27,6 @@ options { tokenVocab=MalloyLexer; }
 
 malloyDocument: (malloyStatement | SEMI)* EOF;
 
-closeCurly
-  : CCURLY
-  // ANTLR VSCode plugin loses it's tiny mind if { } aren't matched
-  // even in the error string below
-  | { this.notifyErrorListeners("'{' missing a '}'"); }
-  ;
-
 malloyStatement
   : defineSourceStatement
   | defineQuery
@@ -133,12 +126,7 @@ connectionId
   : id;
 
 queryProperties
-  : filterShortcut
-  | OCURLY (queryStatement | SEMI)* closeCurly
-  ;
-
-filterShortcut
-  : OCURLY QMARK fieldExpr closeCurly
+  : OCURLY (queryStatement | SEMI)* closeCurly
   ;
 
 queryName : id;
@@ -169,7 +157,6 @@ sourceNameDef: id;
 
 exploreProperties
   : OCURLY (exploreStatement | SEMI)* closeCurly
-  | filterShortcut
   ;
 
 exploreStatement
@@ -331,7 +318,7 @@ queryExtendStatement
   ;
 
 queryExtendStatementList
-  : OCURLY (queryExtendStatement | SEMI)* closeCurly
+  : OCURLY (queryExtendStatement | SEMI)* CCURLY
   ;
 
 joinList
@@ -688,7 +675,7 @@ starQualified
   : OCURLY (
       (EXCEPT fieldNameList)
     | COMMA
-  )+ closeCurly
+  )+ CCURLY
   ;
 
 taggedRef
@@ -726,3 +713,13 @@ debugPartial: partialAllowedFieldExpr EOF;
 experimentalStatementForTesting // this only exists to enable tests for the experimental compiler flag
   : SEMI SEMI OBRACK string CBRACK
   ;
+
+// Try to show a nice error for a missing }.  Only use this when the next
+// legal symbols after the curly are things which would be illegal inside
+// the curly brackets.
+closeCurly
+  : CCURLY
+  // ANTLR VSCode plugin loses it's tiny mind if { } aren't matched
+  // even in the error string below
+  | { this.notifyErrorListeners("'{' missing a '}'"); }
+  ;
diff --git a/packages/malloy/src/lang/malloy-to-ast.ts b/packages/malloy/src/lang/malloy-to-ast.ts
@@ -282,16 +282,6 @@ export class MalloyToAST
     );
   }
 
-  protected getFilterShortcut(cx: parse.FilterShortcutContext): ast.Filter {
-    const el = this.getFilterElement(cx.fieldExpr());
-    this.m4advisory(
-      cx,
-      'filter-shortcut',
-      'Filter shortcut `{? condition }` is deprecated; use `{ where: condition } instead'
-    );
-    return new ast.Filter([el]);
-  }
-
   protected getPlainStringFrom(cx: HasString): string {
     const [result, errors] = getPlainString(cx);
     for (const error of errors) {
@@ -444,16 +434,12 @@ export class MalloyToAST
   }
 
   visitExploreProperties(pcx: parse.ExplorePropertiesContext): ast.SourceDesc {
-    const filterCx = pcx.filterShortcut();
     const visited = this.only<ast.SourceProperty>(
       pcx.exploreStatement().map(ecx => this.visit(ecx)),
       x => ast.isSourceProperty(x) && x,
       'source property'
     );
     const propList = new ast.SourceDesc(visited);
-    if (filterCx) {
-      propList.push(this.getFilterShortcut(filterCx));
-    }
     return propList;
   }
 
@@ -848,10 +834,6 @@ export class MalloyToAST
       x => ast.isQueryProperty(x) && x,
       'query statement'
     );
-    const fcx = pcx.filterShortcut();
-    if (fcx) {
-      qProps.push(this.getFilterShortcut(fcx));
-    }
     return new ast.QOpDesc(qProps);
   }
 

diff --git a/packages/malloy/src/lang/parse-log.ts b/packages/malloy/src/lang/parse-log.ts
@@ -336,7 +336,6 @@ type MessageParameterTypes = {
   'foreign-key-in-join-cross': string;
   'expression-type-error': string;
   'unexpected-statement-in-translation': string;
-  'filter-shortcut': string;
   'illegal-query-interpolation-outside-sql-block': string;
   'percent-terminated-query-interpolation': string;
   'failed-to-parse-time-literal': string;

diff --git a/packages/malloy/src/lang/test/query.spec.ts b/packages/malloy/src/lang/test/query.spec.ts
@@ -113,7 +113,7 @@ describe('query:', () => {
     });
     test('query with shortcut filtered turtle', () => {
       expect(`##! -m4warnings
-        query: allA is ab -> aturtle + {? astr ~ 'a%' }`).toTranslate();
+        query: allA is ab -> aturtle + {where: astr ~ 'a%' }`).toTranslate();
     });
     test('query with filtered turtle', () => {
       expect(

diff --git a/packages/malloy/src/lang/test/source.spec.ts b/packages/malloy/src/lang/test/source.spec.ts
@@ -41,16 +41,6 @@ describe('source:', () => {
       "source: xA is _db_.table('aTable') extend { where: astr ~ 'a%' }"
     ).toTranslate();
   });
-  test('shorcut fitlered table m4warning', () => {
-    expect(`
-      ##! m4warnings=warn
-      source: xA is _db_.table('aTable') extend {? astr ~ 'a%' }
-    `).toLog(
-      warningMessage(
-        'Filter shortcut `{? condition }` is deprecated; use `{ where: condition } instead'
-      )
-    );
-  });
   test('fitlered table', () => {
     expect(
       "source: testA is _db_.table('aTable') extend { where: astr ~ 'a%' }"
@@ -558,18 +548,6 @@ describe('source:', () => {
         }
       `).toTranslate();
     });
-    test('refined explore-query m4warning', () => {
-      expect(`
-        ##! m4warnings=warn
-        source: abNew is ab extend {
-          view: for1 is aturtle + {? ai = 1 }
-        }
-      `).toLog(
-        warningMessage(
-          'Filter shortcut `{? condition }` is deprecated; use `{ where: condition } instead'
-        )
-      );
-    });
     test('chained explore-query', () => {
       expect(`
         source: c is a extend {