From b3338ca8dd94edad4180808083f97ef67cb763e6 Mon Sep 17 00:00:00 2001 From: Michael Toy <66150587+mtoy-googly-moogly@users.noreply.github.com> Date: Tue, 31 Dec 2024 06:59:50 -1000 Subject: [PATCH] Tweak handling of missing } (#1985) * remove m4 filter shortcut * remove tests too * Update usage of closeCurly * fix another test --- .../malloy/src/lang/grammar/MalloyParser.g4 | 29 +++++++++---------- packages/malloy/src/lang/malloy-to-ast.ts | 18 ------------ packages/malloy/src/lang/parse-log.ts | 1 - packages/malloy/src/lang/test/query.spec.ts | 2 +- packages/malloy/src/lang/test/source.spec.ts | 22 -------------- 5 files changed, 14 insertions(+), 58 deletions(-) diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4 index c19a38eba..4672962a3 100644 --- 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 '}'"); } + ; \ No newline at end of file diff --git a/packages/malloy/src/lang/malloy-to-ast.ts b/packages/malloy/src/lang/malloy-to-ast.ts index 5accfd87f..b26ddf7ef 100644 --- 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( 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 index a8128049b..245617463 100644 --- 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 index bce500906..b312fd0f8 100644 --- 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 index 7f1a941d0..9b3ad5c92 100644 --- 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 {