From bf787daaea80c66b78574484d45907a8d39a4049 Mon Sep 17 00:00:00 2001 From: Chris Swenson Date: Thu, 12 Dec 2024 14:25:48 -0600 Subject: [PATCH 1/4] Fix issue where joins were causing incorrect usage to be calculated (#2046) --- .../src/model/composite_source_utils.ts | 6 ++- .../databases/all/composite_sources.spec.ts | 41 +++++++++++++++++++ 2 files changed, 46 insertions(+), 1 deletion(-) diff --git a/packages/malloy/src/model/composite_source_utils.ts b/packages/malloy/src/model/composite_source_utils.ts index 60205f6c0e..afa4a4a352 100644 --- a/packages/malloy/src/model/composite_source_utils.ts +++ b/packages/malloy/src/model/composite_source_utils.ts @@ -398,6 +398,10 @@ function compositeFieldUsageWithoutNonCompositeFields( fields: compositeFieldUsage.fields.filter(f => isCompositeField(sourceFieldsByName[f]) ), - joinedUsage: compositeFieldUsage.joinedUsage, + // Today it is not possible for a join to be composite, so we can safely throw + // away all join usage here...; if we ever allow joins in composite source + // inputs, then this will need to be updated to be the joinUsage with joins + // that are not composite filtered out... + joinedUsage: {}, }; } diff --git a/test/src/databases/all/composite_sources.spec.ts b/test/src/databases/all/composite_sources.spec.ts index 1cdace45b0..9e0428b120 100644 --- a/test/src/databases/all/composite_sources.spec.ts +++ b/test/src/databases/all/composite_sources.spec.ts @@ -266,4 +266,45 @@ describe.each(runtimes.runtimeList)('%s', (databaseName, runtime) => { run: x -> { aggregate: foo; group_by: bar, arr.each } `).malloyResultMatches(runtime, {foo: 0}); }); + it('complex nesting composite without join', async () => { + await expect(` + ##! experimental.composite_sources + source: state_facts is ${databaseName}.table('malloytest.state_facts') + source: x is compose( + compose( + state_facts, + state_facts extend { + dimension: bar is 1 + } + ) extend { + measure: foo is sum(0) + }, + state_facts extend { + measure: foo is sum(0) + 1 + dimension: bar is 2 + } + ) + run: x -> { aggregate: foo; group_by: bar } + `).malloyResultMatches(runtime, {foo: 0, bar: 1}); + }); + it('complex nesting composite with join', async () => { + await expect(` + ##! experimental.composite_sources + source: state_facts is ${databaseName}.table('malloytest.state_facts') + source: x is compose( + compose( + state_facts, + state_facts extend { + dimension: the_state is 'CA' + } + ), + state_facts extend { + dimension: the_state is 'IL' + } + ) extend { + join_one: state_facts on the_state = state_facts.state + } + run: x -> { group_by: state_facts.state } + `).malloyResultMatches(runtime, {state: 'CA'}); + }); }); From 9e7085e41aff05e51e9e05e000910221f71eef85 Mon Sep 17 00:00:00 2001 From: Chris Swenson Date: Thu, 12 Dec 2024 16:22:24 -0600 Subject: [PATCH 2/4] Access modifiers and `include` experiment (#2020) * Access modifiers * Fix issue where view could not use protected field; make it work for composite sources * Lint fix * Add star syntax * Change word to internal * Add public * Add definition-time access labels * Lint fix * Update syntax highlighting * Remove in-extend access modifier labels, add `include` block * Implement adding annotations in include block * Notes should inherit, not extend when using include * Tweaks * Update malloy.monarch.ts * Update malloy.tmGrammar.json --- .../grammars/malloy/malloy.monarch.ts | 4 + .../grammars/malloy/malloy.tmGrammar.json | 16 ++ .../test/visual/index.html | 4 +- .../ast/expressions/constant-expression.ts | 4 + .../src/lang/ast/field-space/dynamic-space.ts | 42 ++- .../lang/ast/field-space/parameter-space.ts | 4 + .../lang/ast/field-space/query-input-space.ts | 7 +- .../src/lang/ast/field-space/query-spaces.ts | 6 +- .../src/lang/ast/field-space/refined-space.ts | 83 +++++- .../src/lang/ast/field-space/static-space.ts | 22 ++ packages/malloy/src/lang/ast/index.ts | 1 + .../lang/ast/query-items/field-declaration.ts | 8 + .../lang/ast/query-items/field-references.ts | 14 +- .../ast/query-properties/declare-fields.ts | 10 +- .../ast/source-elements/composite-source.ts | 12 +- .../ast/source-elements/refined-source.ts | 199 ++++++++++++- .../src/lang/ast/source-properties/join.ts | 14 +- .../src/lang/ast/source-properties/renames.ts | 16 ++ .../view-field-declaration.ts | 4 + .../src/lang/ast/source-properties/views.ts | 12 + .../ast/source-query-elements/include-item.ts | 73 +++++ .../ast/source-query-elements/sq-extend.ts | 6 +- .../malloy/src/lang/ast/types/field-space.ts | 1 + .../malloy/src/lang/ast/types/space-entry.ts | 1 + .../lang/ast/view-elements/reference-view.ts | 5 +- .../malloy/src/lang/grammar/MalloyLexer.g4 | 7 + .../malloy/src/lang/grammar/MalloyParser.g4 | 106 +++++-- packages/malloy/src/lang/malloy-to-ast.ts | 192 +++++++++++-- packages/malloy/src/lang/parse-log.ts | 14 + .../malloy/src/lang/test/annotation.spec.ts | 157 ++++++++++ .../lang/test/composite-field-usage.spec.ts | 71 +++++ packages/malloy/src/lang/test/source.spec.ts | 268 ++++++++++++++++++ .../src/model/composite_source_utils.ts | 4 +- packages/malloy/src/model/malloy_types.ts | 6 + 34 files changed, 1327 insertions(+), 66 deletions(-) create mode 100644 packages/malloy/src/lang/ast/source-query-elements/include-item.ts diff --git a/packages/malloy-syntax-highlight/grammars/malloy/malloy.monarch.ts b/packages/malloy-syntax-highlight/grammars/malloy/malloy.monarch.ts index 8eb0b31b07..14347791b1 100644 --- a/packages/malloy-syntax-highlight/grammars/malloy/malloy.monarch.ts +++ b/packages/malloy-syntax-highlight/grammars/malloy/malloy.monarch.ts @@ -181,6 +181,10 @@ export const monarch: Monaco.IMonarchLanguage = { [/\bwhen\b/, 'keyword.other.when'], [/\bpick\b/, 'keyword.other.pick'], [/\bimport\b/, 'keyword.control.import'], + [/\binternal\b/, 'keyword.control.internal'], + [/\bpublic\b/, 'keyword.control.public'], + [/\bprivate\b/, 'keyword.control.private'], + [/\binclude\b/, 'keyword.control.include'], ], properties: [ [/\baccept\b/, 'keyword.control.accept'], diff --git a/packages/malloy-syntax-highlight/grammars/malloy/malloy.tmGrammar.json b/packages/malloy-syntax-highlight/grammars/malloy/malloy.tmGrammar.json index 6b1aa425cf..b2af76414d 100644 --- a/packages/malloy-syntax-highlight/grammars/malloy/malloy.tmGrammar.json +++ b/packages/malloy-syntax-highlight/grammars/malloy/malloy.tmGrammar.json @@ -521,6 +521,22 @@ { "match": "(?i)\\bimport\\b", "name": "keyword.control.import" + }, + { + "match": "(?i)\\binternal\\b", + "name": "keyword.control.internal" + }, + { + "match": "(?i)\\bpublic\\b", + "name": "keyword.control.public" + }, + { + "match": "(?i)\\bprivate\\b", + "name": "keyword.control.private" + }, + { + "match": "(?i)\\binclude\\b", + "name": "keyword.control.include" } ] } diff --git a/packages/malloy-syntax-highlight/test/visual/index.html b/packages/malloy-syntax-highlight/test/visual/index.html index a040712647..2329ec029f 100644 --- a/packages/malloy-syntax-highlight/test/visual/index.html +++ b/packages/malloy-syntax-highlight/test/visual/index.html @@ -43,7 +43,7 @@ }
- + - \ No newline at end of file + diff --git a/packages/malloy/src/lang/ast/expressions/constant-expression.ts b/packages/malloy/src/lang/ast/expressions/constant-expression.ts index 80861a6735..d78aa8c543 100644 --- a/packages/malloy/src/lang/ast/expressions/constant-expression.ts +++ b/packages/malloy/src/lang/ast/expressions/constant-expression.ts @@ -62,6 +62,10 @@ export class ConstantFieldSpace implements FieldSpace { isQueryFieldSpace(): this is QueryFieldSpace { return false; } + + isProtectedAccessSpace(): boolean { + return false; + } } export class ConstantExpression extends ExpressionDef { diff --git a/packages/malloy/src/lang/ast/field-space/dynamic-space.ts b/packages/malloy/src/lang/ast/field-space/dynamic-space.ts index 0d8aec4c3c..66f82dec53 100644 --- a/packages/malloy/src/lang/ast/field-space/dynamic-space.ts +++ b/packages/malloy/src/lang/ast/field-space/dynamic-space.ts @@ -47,6 +47,8 @@ export abstract class DynamicSpace private complete = false; private parameters: HasParameter[] = []; protected newTimezone?: string; + protected newAccessModifiers = new Map(); + protected newNotes = new Map(); constructor(extending: SourceDef) { super(structuredClone(extending), extending.dialect); @@ -110,6 +112,7 @@ export abstract class DynamicSpace this.sourceDef = {...this.fromSource, fields: []}; this.sourceDef.parameters = parameters; + const fieldIndices = new Map(); // Need to process the entities in specific order const fields: [string, SpaceField][] = []; const joins: [string, SpaceField][] = []; @@ -126,16 +129,18 @@ export abstract class DynamicSpace } const reorderFields = [...fields, ...joins, ...turtles]; const parameterSpace = this.parameterSpace(); - for (const [, field] of reorderFields) { + for (const [name, field] of reorderFields) { if (field instanceof JoinSpaceField) { const joinStruct = field.join.structDef(parameterSpace); if (!ErrorFactory.didCreate(joinStruct)) { + fieldIndices.set(name, this.sourceDef.fields.length); this.sourceDef.fields.push(joinStruct); fixupJoins.push([field.join, joinStruct]); } } else { const fieldDef = field.fieldDef(); if (fieldDef) { + fieldIndices.set(name, this.sourceDef.fields.length); this.sourceDef.fields.push(fieldDef); } // TODO I'm just removing this, but perhaps instead I should just filter @@ -150,6 +155,41 @@ export abstract class DynamicSpace for (const [join, missingOn] of fixupJoins) { join.fixupJoinOn(this, missingOn); } + // Add access modifiers at the end so views don't obey them + for (const [name, access] of this.newAccessModifiers) { + const index = this.sourceDef.fields.findIndex( + f => f.as ?? f.name === name + ); + if (index === -1) { + throw new Error(`Can't find field '${name}' to set access modifier`); + } + if (access === 'public') { + delete this.sourceDef.fields[index].accessModifier; + } else { + this.sourceDef.fields[index] = { + ...this.sourceDef.fields[index], + accessModifier: access, + }; + } + } + // TODO does this need to be done when the space is instantiated? + // e.g. if a field had a compiler flag on it... + for (const [name, note] of this.newNotes) { + const index = this.sourceDef.fields.findIndex( + f => f.as ?? f.name === name + ); + if (index === -1) { + throw new Error(`Can't find field '${name}' to set access modifier`); + } + const field = this.sourceDef.fields[index]; + this.sourceDef.fields[index] = { + ...field, + annotation: { + ...note, + inherits: field.annotation, + }, + }; + } } if (this.newTimezone && model.isSourceDef(this.sourceDef)) { this.sourceDef.queryTimezone = this.newTimezone; diff --git a/packages/malloy/src/lang/ast/field-space/parameter-space.ts b/packages/malloy/src/lang/ast/field-space/parameter-space.ts index 44524b0b9f..3060902265 100644 --- a/packages/malloy/src/lang/ast/field-space/parameter-space.ts +++ b/packages/malloy/src/lang/ast/field-space/parameter-space.ts @@ -90,4 +90,8 @@ export class ParameterSpace implements FieldSpace { isQueryFieldSpace(): this is QueryFieldSpace { return false; } + + isProtectedAccessSpace(): boolean { + return false; + } } diff --git a/packages/malloy/src/lang/ast/field-space/query-input-space.ts b/packages/malloy/src/lang/ast/field-space/query-input-space.ts index 2f76c1c325..5df7967090 100644 --- a/packages/malloy/src/lang/ast/field-space/query-input-space.ts +++ b/packages/malloy/src/lang/ast/field-space/query-input-space.ts @@ -47,7 +47,8 @@ export class QueryInputSpace extends RefinedSpace implements QueryFieldSpace { */ constructor( input: SourceDef, - private queryOutput: QueryOperationSpace + private queryOutput: QueryOperationSpace, + public readonly _isProtectedAccessSpace: boolean ) { super(input); } @@ -72,4 +73,8 @@ export class QueryInputSpace extends RefinedSpace implements QueryFieldSpace { inputSpace() { return this; } + + isProtectedAccessSpace(): boolean { + return this._isProtectedAccessSpace; + } } diff --git a/packages/malloy/src/lang/ast/field-space/query-spaces.ts b/packages/malloy/src/lang/ast/field-space/query-spaces.ts index fb12006b42..0c9f5bc560 100644 --- a/packages/malloy/src/lang/ast/field-space/query-spaces.ts +++ b/packages/malloy/src/lang/ast/field-space/query-spaces.ts @@ -98,7 +98,11 @@ export abstract class QueryOperationSpace ) { super(queryInputSpace.emptyStructDef()); - this.exprSpace = new QueryInputSpace(queryInputSpace.structDef(), this); + this.exprSpace = new QueryInputSpace( + queryInputSpace.structDef(), + this, + queryInputSpace.isProtectedAccessSpace() + ); if (refineThis) this.addRefineFromFields(refineThis); } diff --git a/packages/malloy/src/lang/ast/field-space/refined-space.ts b/packages/malloy/src/lang/ast/field-space/refined-space.ts index 8f663daebf..d354678103 100644 --- a/packages/malloy/src/lang/ast/field-space/refined-space.ts +++ b/packages/malloy/src/lang/ast/field-space/refined-space.ts @@ -21,12 +21,20 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ -import {SourceDef} from '../../../model/malloy_types'; +import { + AccessModifierLabel, + Annotation, + DocumentLocation, + SourceDef, +} from '../../../model/malloy_types'; import {FieldListEdit} from '../source-properties/field-list-edit'; import {DynamicSpace} from './dynamic-space'; import {canMakeEntry} from '../types/space-entry'; import {MalloyElement} from '../types/malloy-element'; import {ParameterSpace} from './parameter-space'; +import {FieldReference} from '../query-items/field-references'; +import {RenameSpaceField} from './rename-space-field'; +import {SpaceField} from '../types/space-field'; export class RefinedSpace extends DynamicSpace { /** @@ -37,10 +45,65 @@ export class RefinedSpace extends DynamicSpace { static filteredFrom( from: SourceDef, choose: FieldListEdit | undefined, + fieldsToInclude: Set | undefined, + renames: + | { + as: string; + name: FieldReference; + location: DocumentLocation; + }[] + | undefined, parameters: ParameterSpace | undefined ): RefinedSpace { const edited = new RefinedSpace(from); - if (choose) { + const renameMap = new Map< + string, + {as: string; location: DocumentLocation; logTo: MalloyElement} + >(); + for (const rename of renames ?? []) { + if (renameMap.has(rename.name.refString)) { + rename.name.logError( + 'already-renamed', + `${rename.name.refString} already renamed to ${rename.as}` + ); + } else { + renameMap.set(rename.name.refString, { + as: rename.as, + location: rename.location, + logTo: rename.name, + }); + } + } + if (fieldsToInclude !== undefined) { + const oldMap = edited.entries(); + edited.dropEntries(); + for (const [symbol, value] of oldMap) { + if (fieldsToInclude.has(symbol)) { + const renamed = renameMap.get(symbol); + if (renamed) { + if (value instanceof SpaceField) { + edited.setEntry( + renamed.as, + new RenameSpaceField(value, renamed.as, renamed.location) + ); + } else { + renamed.logTo.logError( + 'cannot-rename-non-field', + `Cannot rename \`${symbol}\` which is not a field` + ); + } + } else { + edited.setEntry(symbol, value); + } + } + } + if (choose !== undefined) { + choose.logError( + 'accept-except-not-compatible-with-include', + "Can't use `accept:` or `except:` with `include`" + ); + } + } else if (choose) { const names = choose.refs.list; const oldMap = edited.entries(); for (const name of names) { @@ -87,4 +150,20 @@ export class RefinedSpace extends DynamicSpace { ); } } + + addAccessModifiers(ams: Map): void { + for (const [symbol, am] of ams) { + this.newAccessModifiers.set(symbol, am); + } + } + + addNotes(notes: Map): void { + for (const [symbol, note] of notes) { + this.newNotes.set(symbol, note); + } + } + + isProtectedAccessSpace(): boolean { + return true; + } } diff --git a/packages/malloy/src/lang/ast/field-space/static-space.ts b/packages/malloy/src/lang/ast/field-space/static-space.ts index 3b7fb5ff97..93e6129e37 100644 --- a/packages/malloy/src/lang/ast/field-space/static-space.ts +++ b/packages/malloy/src/lang/ast/field-space/static-space.ts @@ -103,6 +103,10 @@ export class StaticSpace implements FieldSpace { return this.memoMap; } + isProtectedAccessSpace(): boolean { + return false; + } + protected dropEntries(): void { this.memoMap = {}; } @@ -174,6 +178,24 @@ export class StaticSpace implements FieldSpace { text: head.refString, }); } + if (definition?.accessModifier) { + // TODO path.length === 1 will not work with namespaces + if ( + !( + this.isProtectedAccessSpace() && + definition.accessModifier === 'internal' && + path.length === 1 + ) + ) { + return { + error: { + message: `'${head}' is ${definition?.accessModifier}`, + code: 'field-not-accessible', + }, + found: undefined, + }; + } + } } // cswenson review todo { else this is SpaceEntry not a field which can only be a param and what is going on? } const joinPath = found instanceof StructSpaceFieldBase diff --git a/packages/malloy/src/lang/ast/index.ts b/packages/malloy/src/lang/ast/index.ts index e23d1b4c28..544cd350fc 100644 --- a/packages/malloy/src/lang/ast/index.ts +++ b/packages/malloy/src/lang/ast/index.ts @@ -33,6 +33,7 @@ export * from './source-query-elements/sq-source'; export * from './source-query-elements/sq-reference'; export * from './source-query-elements/sq-extend'; export * from './source-query-elements/sq-compose'; +export * from './source-query-elements/include-item'; export * from './source-properties/field-list-edit'; export * from './source-properties/primary-key'; diff --git a/packages/malloy/src/lang/ast/query-items/field-declaration.ts b/packages/malloy/src/lang/ast/query-items/field-declaration.ts index b8a65f9e55..0bf2582880 100644 --- a/packages/malloy/src/lang/ast/query-items/field-declaration.ts +++ b/packages/malloy/src/lang/ast/query-items/field-declaration.ts @@ -77,6 +77,10 @@ export abstract class AtomicFieldDeclaration super({expr: expr}); } + getName(): string { + return this.defineName; + } + fieldDef(fs: FieldSpace, exprName: string): FieldDef { /* * In an explore we cannot reference the thing we are defining, you need @@ -287,6 +291,10 @@ export class DefSpace implements FieldSpace { } throw new Error('Not a query field space'); } + + isProtectedAccessSpace(): boolean { + return true; + } } export class FieldDefinitionValue extends SpaceField { diff --git a/packages/malloy/src/lang/ast/query-items/field-references.ts b/packages/malloy/src/lang/ast/query-items/field-references.ts index 0785ebae28..4b00df09c1 100644 --- a/packages/malloy/src/lang/ast/query-items/field-references.ts +++ b/packages/malloy/src/lang/ast/query-items/field-references.ts @@ -78,6 +78,10 @@ export abstract class FieldReference } } + getName(): string { + return this.nameString; + } + get refToField(): RefToField { return { type: 'fieldref', @@ -134,6 +138,14 @@ export class AcceptExceptFieldReference extends FieldReference { } } +export class AccessModifierFieldReference extends FieldReference { + elementType = 'accessModifierFieldReference'; + // Nothing to typecheck here + typecheck() { + return; + } +} + export class ExpressionFieldReference extends FieldReference { elementType = 'expressionFieldReference'; // We assume that the outer expression will typecheck this @@ -236,7 +248,7 @@ export class WildcardFieldReference extends MalloyElement implements Noteable { except = new Set(); constructor(readonly joinPath: FieldReference | undefined) { super(); - this.has({joinPath: joinPath}); + this.has({joinPath}); } getFieldDef(): FieldDef { diff --git a/packages/malloy/src/lang/ast/query-properties/declare-fields.ts b/packages/malloy/src/lang/ast/query-properties/declare-fields.ts index 7ea6e8b939..44ea3f726d 100644 --- a/packages/malloy/src/lang/ast/query-properties/declare-fields.ts +++ b/packages/malloy/src/lang/ast/query-properties/declare-fields.ts @@ -21,6 +21,7 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +import {AccessModifierLabel} from '../../../model'; import {AtomicFieldDeclaration} from '../query-items/field-declaration'; import {DefinitionList} from '../types/definition-list'; import {QueryBuilder} from '../types/query-builder'; @@ -37,7 +38,10 @@ export class DeclareFields queryRefinementStage = LegalRefinementStage.Single; forceQueryClass = undefined; - constructor(fields: AtomicFieldDeclaration[]) { + constructor( + fields: AtomicFieldDeclaration[], + readonly accessModifier: AccessModifierLabel | undefined + ) { super(fields); } @@ -46,4 +50,8 @@ export class DeclareFields executeFor.inputFS.extendSource(qel); } } + + get delarationNames(): string[] { + return this.list.map(el => el.defineName); + } } diff --git a/packages/malloy/src/lang/ast/source-elements/composite-source.ts b/packages/malloy/src/lang/ast/source-elements/composite-source.ts index 5ca6047868..ac10e9b866 100644 --- a/packages/malloy/src/lang/ast/source-elements/composite-source.ts +++ b/packages/malloy/src/lang/ast/source-elements/composite-source.ts @@ -44,7 +44,7 @@ export class CompositeSource extends Source { const dialect = sourceDefs[0].dialect; const name = 'composite_source'; const fields: FieldDef[] = []; - const fieldNames = new Set(); + const fieldsByName = new Map(); this.sources.forEach((source, index) => { const sourceDef = sourceDefs[index]; // Check that connections all match; don't bother checking dialect, since it will @@ -63,9 +63,12 @@ export class CompositeSource extends Source { ); continue; } + if (field.accessModifier === 'private') { + continue; + } const fieldName = field.as ?? field.name; - if (!fieldNames.has(fieldName)) { - fieldNames.add(fieldName); + const existing = fieldsByName.get(fieldName); + if (existing === undefined) { const compositeField: AtomicFieldDef = { ...field, name: fieldName, @@ -75,7 +78,10 @@ export class CompositeSource extends Source { code: this.code, location: this.codeLocation, }; + fieldsByName.set(fieldName, compositeField); fields.push(compositeField); + } else if (field.accessModifier === 'internal') { + existing.accessModifier = 'internal'; } } }); diff --git a/packages/malloy/src/lang/ast/source-elements/refined-source.ts b/packages/malloy/src/lang/ast/source-elements/refined-source.ts index 1bc8a4c41a..b0453cdc63 100644 --- a/packages/malloy/src/lang/ast/source-elements/refined-source.ts +++ b/packages/malloy/src/lang/ast/source-elements/refined-source.ts @@ -22,7 +22,9 @@ */ import { + AccessModifierLabel, Annotation, + DocumentLocation, SourceDef, expressionIsCalculation, } from '../../../model/malloy_types'; @@ -42,6 +44,15 @@ import {Renames} from '../source-properties/renames'; import {MakeEntry} from '../types/space-entry'; import {ParameterSpace} from '../field-space/parameter-space'; import {JoinStatement} from '../source-properties/join'; +import { + IncludeAccessItem, + IncludeExceptItem, + IncludeItem, +} from '../source-query-elements/include-item'; +import { + FieldReference, + WildcardFieldReference, +} from '../query-items/field-references'; /** * A Source made from a source reference and a set of refinements @@ -52,9 +63,13 @@ export class RefinedSource extends Source { constructor( readonly source: Source, - readonly refinement: SourceDesc + readonly refinement: SourceDesc, + readonly includeList: IncludeItem[] | undefined ) { super({source, refinement}); + if (includeList) { + this.has({includeList}); + } } getSourceDef(parameterSpace: ParameterSpace | undefined): SourceDef { @@ -71,6 +86,10 @@ export class RefinedSource extends Source { const filters: Filter[] = []; let newTimezone: string | undefined; + const inlineAccessModifiers: { + access: AccessModifierLabel; + fields: string[]; + }[] = []; for (const el of this.refinement.list) { if (el instanceof ObjectAnnotation) { // Treat lone annotations as comments @@ -98,6 +117,12 @@ export class RefinedSource extends Source { el instanceof Renames ) { fields.push(...el.list); + if (el.accessModifier) { + inlineAccessModifiers.push({ + fields: el.delarationNames, + access: el.accessModifier, + }); + } } else if (el instanceof Filter) { filters.push(el); } else if (el instanceof TimezoneStatement) { @@ -112,13 +137,28 @@ export class RefinedSource extends Source { const paramSpace = pList ? new ParameterSpace(pList) : undefined; const from = structuredClone(this.source.getSourceDef(paramSpace)); + const {fieldsToInclude, modifiers, renames, notes} = processIncludeList( + this.includeList, + from + ); + for (const modifier of inlineAccessModifiers) { + for (const field of modifier.fields) { + modifiers.set(field, modifier.access); + } + } // Note that this is explicitly not: // const from = structuredClone(this.source.withParameters(parameterSpace, pList)); // Because the parameters are added to the resulting struct, not the base struct if (primaryKey) { from.primaryKey = primaryKey.field.name; } - const fs = RefinedSpace.filteredFrom(from, fieldListEdit, paramSpace); + const fs = RefinedSpace.filteredFrom( + from, + fieldListEdit, + fieldsToInclude, + renames, + paramSpace + ); if (newTimezone) { fs.setTimezone(newTimezone); } @@ -132,6 +172,8 @@ export class RefinedSource extends Source { primaryKey.logError(keyDef.error.code, keyDef.error.message); } } + fs.addAccessModifiers(modifiers); + fs.addNotes(notes); const retStruct = fs.structDef(); const filterList = retStruct.filterList || []; @@ -157,3 +199,156 @@ export class RefinedSource extends Source { return retStruct; } } + +function processIncludeList( + includeItems: IncludeItem[] | undefined, + from: SourceDef +) { + // TODO error/warning if you include both star and specific fields with the same modifier... + const allFields = new Set(from.fields.map(f => f.name)); + const alreadyPrivateFields = new Set( + from.fields.filter(f => f.accessModifier === 'private').map(f => f.name) + ); + let mode: 'exclude' | 'include' | undefined = undefined; + const fieldsMentioned = new Set(); + let star: AccessModifierLabel | 'inherit' | undefined = undefined; + let starNote: Annotation | undefined = undefined; + const modifiers = new Map(); + const renames: { + as: string; + name: FieldReference; + location: DocumentLocation; + }[] = []; + const notes = new Map(); + if (includeItems === undefined) { + return {fieldsToInclude: undefined, modifiers, renames, notes}; + } + for (const item of includeItems) { + if (item instanceof IncludeAccessItem) { + for (const f of item.fields) { + if (f.name instanceof WildcardFieldReference) { + if (f.name.joinPath) { + f.logError( + 'unsupported-path-in-include', + 'Wildcards with paths are not supported in `include` blocks' + ); + } + if (star !== undefined) { + item.logError( + 'already-used-star-in-include', + 'Wildcard already used in this include block' + ); + } else { + star = item.kind ?? 'inherit'; + starNote = { + notes: f.note?.notes ?? [], + blockNotes: item.note?.blockNotes ?? [], + }; + } + } else { + if (mode === 'exclude') { + item.logError( + 'include-after-exclude', + 'Cannot include specific fields if specific fields are already excluded' + ); + continue; + } + mode = 'include'; + const name = f.name.refString; + if (alreadyPrivateFields.has(name)) { + f.logError( + 'cannot-expand-access', + `Cannot expand access of \`${name}\` from private to ${item.kind}` + ); + } + if (modifiers.has(name)) { + f.logError( + 'duplicate-include', + `Field \`${name}\` already referenced in include list` + ); + } else { + if (item.kind !== undefined) { + modifiers.set(name, item.kind); + } + fieldsMentioned.add(name); + if (f.note || item.note) { + notes.set(name, { + notes: f.note?.notes ?? [], + blockNotes: item.note?.blockNotes ?? [], + }); + } + } + if (f.as) { + if (f.name instanceof WildcardFieldReference) { + f.logError( + 'wildcard-include-rename', + 'Cannot rename a wildcard field in an `include` block' + ); + } else { + renames.push({ + name: f.name, + as: f.as, + location: f.location, + }); + } + } + } + } + } else if (item instanceof IncludeExceptItem) { + for (const f of item.fields) { + if (f instanceof WildcardFieldReference) { + if (f.joinPath) { + f.logError( + 'unsupported-path-in-include', + 'Wildcards with paths are not supported in `include` blocks' + ); + } else { + f.logWarning( + 'wildcard-except-redundant', + '`except: *` is implied, unless another clause uses *' + ); + } + } else { + if (mode === 'include') { + item.logError( + 'exclude-after-include', + 'Cannot exclude specific fields if specific fields are already included' + ); + } else { + mode = 'exclude'; + star = 'inherit'; + fieldsMentioned.add(f.refString); + } + } + } + } + } + const starFields: Set = new Set(allFields); + fieldsMentioned.forEach(f => starFields.delete(f)); + alreadyPrivateFields.forEach(f => starFields.delete(f)); + let fieldsToInclude: Set; + if (star !== undefined) { + for (const field of starFields) { + if (star !== 'inherit') { + modifiers.set(field, star); + } + if (starNote) { + notes.set(field, {...starNote}); + } + } + } + if (mode !== 'exclude') { + if (star !== undefined) { + fieldsToInclude = allFields; + } else { + fieldsToInclude = fieldsMentioned; + } + } else { + fieldsToInclude = starFields; + } + // TODO: validate that a field isn't renamed more than once + // TODO: validate that excluded fields are not referenced by included fields + // TODO: make renames fields work in existing references + // TODO: make renames that would replace an excluded field don't do that + return {fieldsToInclude, modifiers, renames, notes}; +} diff --git a/packages/malloy/src/lang/ast/source-properties/join.ts b/packages/malloy/src/lang/ast/source-properties/join.ts index 9762392a9a..ed8c473a51 100644 --- a/packages/malloy/src/lang/ast/source-properties/join.ts +++ b/packages/malloy/src/lang/ast/source-properties/join.ts @@ -29,6 +29,7 @@ import { MatrixOperation, SourceDef, isJoinable, + AccessModifierLabel, } from '../../../model/malloy_types'; import {DynamicSpace} from '../field-space/dynamic-space'; import {JoinSpaceField} from '../field-space/join-space-field'; @@ -67,6 +68,10 @@ export abstract class Join ); } + getName(): string { + return this.name.refString; + } + protected getStructDefFromExpr(parameterSpace: ParameterSpace): SourceDef { const source = this.sourceExpr.getSource(); if (!source) { @@ -233,7 +238,10 @@ export class JoinStatement forceQueryClass = undefined; queryRefinementStage = LegalRefinementStage.Single; - constructor(joins: Join[]) { + constructor( + joins: Join[], + readonly accessModifier: AccessModifierLabel | undefined + ) { super(joins); } @@ -243,4 +251,8 @@ export class JoinStatement executeFor.alwaysJoins.push(qel.name.refString); } } + + get delarationNames(): string[] { + return this.list.map(el => el.name.refString); + } } diff --git a/packages/malloy/src/lang/ast/source-properties/renames.ts b/packages/malloy/src/lang/ast/source-properties/renames.ts index cbc995d784..6fdb4c943d 100644 --- a/packages/malloy/src/lang/ast/source-properties/renames.ts +++ b/packages/malloy/src/lang/ast/source-properties/renames.ts @@ -21,6 +21,7 @@ * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. */ +import {AccessModifierLabel} from '../../../model'; import {DynamicSpace} from '../field-space/dynamic-space'; import {RenameSpaceField} from '../field-space/rename-space-field'; import {FieldName} from '../types/field-space'; @@ -63,8 +64,23 @@ export class RenameField extends MalloyElement implements MakeEntry { ); } } + + getName(): string { + return this.newName; + } } export class Renames extends ListOf { elementType = 'renameFields'; + + constructor( + fields: RenameField[], + readonly accessModifier: AccessModifierLabel | undefined + ) { + super(fields); + } + + get delarationNames(): string[] { + return this.list.map(el => el.getName()); + } } diff --git a/packages/malloy/src/lang/ast/source-properties/view-field-declaration.ts b/packages/malloy/src/lang/ast/source-properties/view-field-declaration.ts index de9aed075a..a5b8cee045 100644 --- a/packages/malloy/src/lang/ast/source-properties/view-field-declaration.ts +++ b/packages/malloy/src/lang/ast/source-properties/view-field-declaration.ts @@ -52,6 +52,10 @@ export class ViewFieldDeclaration fs.newEntry(this.name, this, qf); } + getName(): string { + return this.name; + } + getFieldDef(fs: FieldSpace): model.TurtleDef { const {pipeline, annotation} = this.view.pipelineComp(fs); const checkedPipeline = detectAndRemovePartialStages(pipeline, this); diff --git a/packages/malloy/src/lang/ast/source-properties/views.ts b/packages/malloy/src/lang/ast/source-properties/views.ts index 6c8ecdf557..7f934f07ab 100644 --- a/packages/malloy/src/lang/ast/source-properties/views.ts +++ b/packages/malloy/src/lang/ast/source-properties/views.ts @@ -23,7 +23,19 @@ import {ViewFieldDeclaration} from './view-field-declaration'; import {DefinitionList} from '../types/definition-list'; +import {AccessModifierLabel} from '../../../model'; export class Views extends DefinitionList { elementType = 'turtleDefList'; + + constructor( + views: ViewFieldDeclaration[], + readonly accessModifier: AccessModifierLabel | undefined + ) { + super(views); + } + + get delarationNames(): string[] { + return this.list.map(el => el.name); + } } diff --git a/packages/malloy/src/lang/ast/source-query-elements/include-item.ts b/packages/malloy/src/lang/ast/source-query-elements/include-item.ts new file mode 100644 index 0000000000..365127cf01 --- /dev/null +++ b/packages/malloy/src/lang/ast/source-query-elements/include-item.ts @@ -0,0 +1,73 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import {MalloyElement} from '../types/malloy-element'; +import { + AccessModifierFieldReference, + FieldReference, + WildcardFieldReference, +} from '../query-items/field-references'; +import {Annotation} from '../../../model'; +import {extendNoteMethod, Noteable} from '../types/noteable'; + +// type RenameSpec = { +// as: string; +// name: FieldReference; +// location: DocumentLocation; +// }; + +export abstract class IncludeItem extends MalloyElement { + abstract kind: 'private' | 'public' | 'internal' | 'except' | undefined; +} + +export class IncludeAccessItem extends IncludeItem implements Noteable { + elementType = 'include-access-item'; + readonly isNoteableObj = true; + extendNote = extendNoteMethod; + note?: Annotation; + constructor( + readonly kind: 'private' | 'public' | 'internal' | undefined, + readonly fields: IncludeListItem[] + ) { + super(); + this.has({fields}); + } + + // getRenames(): RenameSpec[] { + // const renames: RenameSpec[] = []; + // if (this.fields === '*') return renames; + // for (const item of this.fields) { + // if (item.as) { + // renames.push({as: item.as, name: item.name, location: item.location}); + // } + // } + // return renames; + // } +} + +export class IncludeExceptItem extends IncludeItem { + elementType = 'include-except-item'; + kind = 'except' as const; + constructor(readonly fields: (FieldReference | WildcardFieldReference)[]) { + super(); + this.has({fields}); + } +} + +export class IncludeListItem extends MalloyElement implements Noteable { + elementType = 'include-list-item'; + readonly isNoteableObj = true; + extendNote = extendNoteMethod; + note?: Annotation; + + constructor( + readonly name: AccessModifierFieldReference | WildcardFieldReference, + readonly as: string | undefined + ) { + super({name}); + } +} diff --git a/packages/malloy/src/lang/ast/source-query-elements/sq-extend.ts b/packages/malloy/src/lang/ast/source-query-elements/sq-extend.ts index 709398453f..b473a816bc 100644 --- a/packages/malloy/src/lang/ast/source-query-elements/sq-extend.ts +++ b/packages/malloy/src/lang/ast/source-query-elements/sq-extend.ts @@ -24,6 +24,7 @@ import {RefinedSource} from '../source-elements/refined-source'; import {SourceQueryElement} from './source-query-element'; import {SourceDesc} from '../types/source-desc'; +import {IncludeItem} from './include-item'; /** * An element which represents adding source extensions to a @@ -38,7 +39,8 @@ export class SQExtend extends SourceQueryElement { constructor( readonly sqSrc: SourceQueryElement, - readonly extend: SourceDesc + readonly extend: SourceDesc, + readonly includeList: IncludeItem[] | undefined ) { super({sqSrc, extend}); } @@ -49,7 +51,7 @@ export class SQExtend extends SourceQueryElement { } const src = this.sqSrc.getSource(); if (src) { - this.asSource = new RefinedSource(src, this.extend); + this.asSource = new RefinedSource(src, this.extend, this.includeList); this.has({asSource: this.asSource}); return this.asSource; } diff --git a/packages/malloy/src/lang/ast/types/field-space.ts b/packages/malloy/src/lang/ast/types/field-space.ts index 93ef0d9509..6c94063fb5 100644 --- a/packages/malloy/src/lang/ast/types/field-space.ts +++ b/packages/malloy/src/lang/ast/types/field-space.ts @@ -42,6 +42,7 @@ export interface FieldSpace { dialectObj(): Dialect | undefined; dialectName(): string; isQueryFieldSpace(): this is QueryFieldSpace; + isProtectedAccessSpace(): boolean; } export interface SourceFieldSpace extends FieldSpace { diff --git a/packages/malloy/src/lang/ast/types/space-entry.ts b/packages/malloy/src/lang/ast/types/space-entry.ts index 4d290e2381..4e070e8f36 100644 --- a/packages/malloy/src/lang/ast/types/space-entry.ts +++ b/packages/malloy/src/lang/ast/types/space-entry.ts @@ -32,6 +32,7 @@ export abstract class SpaceEntry { export interface MakeEntry extends MalloyElement { makeEntry: (fs: DynamicSpace) => void; + getName(): string; } export function canMakeEntry( diff --git a/packages/malloy/src/lang/ast/view-elements/reference-view.ts b/packages/malloy/src/lang/ast/view-elements/reference-view.ts index c69f982608..aa34a6ac48 100644 --- a/packages/malloy/src/lang/ast/view-elements/reference-view.ts +++ b/packages/malloy/src/lang/ast/view-elements/reference-view.ts @@ -76,10 +76,7 @@ export class ReferenceView extends View { }; }; if (!lookup.found) { - this.logError( - 'view-not-found', - `\`${this.reference.refString}\` is not defined` - ); + this.reference.logError(lookup.error.code, lookup.error.message); return oops(); } if (!(lookup.found instanceof SpaceField)) { diff --git a/packages/malloy/src/lang/grammar/MalloyLexer.g4 b/packages/malloy/src/lang/grammar/MalloyLexer.g4 index e8692dfb3e..bfcd83c22f 100644 --- a/packages/malloy/src/lang/grammar/MalloyLexer.g4 +++ b/packages/malloy/src/lang/grammar/MalloyLexer.g4 @@ -38,6 +38,7 @@ EXTENDQ: E X T E N D SPACE_CHAR* ':'; GROUP_BY: G R O U P '_' B Y SPACE_CHAR* ':'; HAVING: H A V I N G SPACE_CHAR* ':'; INDEX: I N D E X SPACE_CHAR* ':'; +INTERNAL: I N T E R N A L SPACE_CHAR* ':'; JOIN_CROSS: J O I N '_' C R O S S ':'; JOIN_ONE: J O I N '_' O N E SPACE_CHAR* ':'; JOIN_MANY: J O I N '_' M A N Y SPACE_CHAR* ':'; @@ -47,7 +48,9 @@ NEST: N E S T SPACE_CHAR* ':'; ORDER_BY: O R D E R '_' B Y SPACE_CHAR* ':'; PARTITION_BY: P A R T I T I O N '_' B Y SPACE_CHAR* ':'; PRIMARY_KEY: P R I M A R Y '_' K E Y SPACE_CHAR* ':'; +PRIVATE: P R I V A T E SPACE_CHAR* ':'; PROJECT: P R O J E C T SPACE_CHAR* ':'; +PUBLIC: P U B L I C SPACE_CHAR* ':'; QUERY: Q U E R Y SPACE_CHAR* ':'; RENAME: R E N A M E SPACE_CHAR* ':'; RUN: R U N SPACE_CHAR* ':'; @@ -87,9 +90,11 @@ FROM: F R O M ; HAS: H A S ; HOUR: H O U R S?; IMPORT: I M P O R T; +INCLUDE: I N C L U D E; INNER: I N N E R; IS: I S ; IN: I N ; +INTERNAL_KW: I N T E R N A L ; JSON: J S O N; LAST: L A S T ; LEFT: L E F T ; @@ -105,6 +110,8 @@ NUMBER: N U M B E R; ON: O N ; OR: O R ; PICK: P I C K ; +PRIVATE_KW: P R I V A T E ; +PUBLIC_KW: P U B L I C ; QUARTER: Q U A R T E R S?; RIGHT: R I G H T ; SECOND: S E C O N D S?; diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4 index 2cb095f6aa..f9f138f5ba 100644 --- a/packages/malloy/src/lang/grammar/MalloyParser.g4 +++ b/packages/malloy/src/lang/grammar/MalloyParser.g4 @@ -173,26 +173,39 @@ exploreProperties ; exploreStatement - : defDimensions # defExploreDimension_stub - | defMeasures # defExploreMeasure_stub - | declareStatement # defDeclare_stub - | joinStatement # defJoin_stub - | whereStatement # defExploreWhere_stub - | PRIMARY_KEY fieldName # defExplorePrimaryKey - | RENAME renameList # defExploreRename - | (ACCEPT | EXCEPT) fieldNameList # defExploreEditField - | tags (QUERY | VIEW) subQueryDefList # defExploreQuery - | timezoneStatement # defExploreTimezone - | ANNOTATION+ # defExploreAnnotation - | ignoredModelAnnotations # defIgnoreModel_stub + : defDimensions # defExploreDimension_stub + | defMeasures # defExploreMeasure_stub + | declareStatement # defDeclare_stub + | joinStatement # defJoin_stub + | whereStatement # defExploreWhere_stub + | PRIMARY_KEY fieldName # defExplorePrimaryKey + | accessLabel? RENAME renameList # defExploreRename + | (ACCEPT | EXCEPT) fieldNameList # defExploreEditField + | tags accessLabel? (QUERY | VIEW) subQueryDefList + # defExploreQuery + | timezoneStatement # defExploreTimezone + | ANNOTATION+ # defExploreAnnotation + | ignoredModelAnnotations # defIgnoreModel_stub + ; + + +accessLabel + : PUBLIC_KW + | PRIVATE_KW + | INTERNAL_KW + ; + +accessModifierList + : fieldNameList + | STAR starQualified? ; defMeasures - : tags MEASURE defList + : tags accessLabel? MEASURE defList ; defDimensions - : tags DIMENSION defList + : tags accessLabel? DIMENSION defList ; renameList @@ -215,13 +228,13 @@ fieldNameDef: id; joinNameDef: id; declareStatement - : DECLARE defList + : DECLARE accessLabel? defList ; joinStatement - : tags JOIN_ONE joinList # defJoinOne - | tags JOIN_MANY joinList # defJoinMany - | tags JOIN_CROSS joinList # defJoinCross + : tags accessLabel? JOIN_ONE joinList # defJoinOne + | tags accessLabel? JOIN_MANY joinList # defJoinMany + | tags accessLabel? JOIN_CROSS joinList # defJoinCross ; queryExtend @@ -248,14 +261,55 @@ sourceArgument ; sqExpr - : id sourceArguments? # SQID - | OPAREN sqExpr CPAREN # SQParens - | COMPOSE OPAREN (sqExpr (COMMA sqExpr)*)? CPAREN # SQCompose - | sqExpr PLUS segExpr # SQRefinedQuery - | sqExpr ARROW segExpr # SQArrow - | sqExpr EXTEND exploreProperties # SQExtendedSource - | exploreTable # SQTable - | sqlSource # SQSQL + : id sourceArguments? # SQID + | OPAREN sqExpr CPAREN # SQParens + | COMPOSE OPAREN (sqExpr (COMMA sqExpr)*)? CPAREN # SQCompose + | sqExpr PLUS segExpr # SQRefinedQuery + | sqExpr ARROW segExpr # SQArrow + | sqExpr (INCLUDE includeBlock)? EXTEND exploreProperties # SQExtendedSource + | sqExpr INCLUDE includeBlock # SQInclude + | exploreTable # SQTable + | sqlSource # SQSQL + ; + +includeBlock + : OCURLY (includeItem | SEMI)* closeCurly + ; + +includeItem + : tags accessLabelProp includeList + | includeList + | tags EXCEPT includeExceptList + | orphanedAnnotation + ; + +orphanedAnnotation + : ANNOTATION + ; + +accessLabelProp + : PUBLIC + | PRIVATE + | INTERNAL + ; + +includeExceptList + : includeExceptListItem (COMMA? includeExceptListItem)* COMMA? + ; + +includeExceptListItem + : tags fieldName + | tags collectionWildCard + ; + +includeList + : includeField (COMMA? includeField)* COMMA? + ; + +includeField + : tags (as=fieldName isDefine)? name=fieldName + | tags name=fieldName + | tags collectionWildCard ; segExpr diff --git a/packages/malloy/src/lang/malloy-to-ast.ts b/packages/malloy/src/lang/malloy-to-ast.ts index 9c971d27d2..883f2d8487 100644 --- a/packages/malloy/src/lang/malloy-to-ast.ts +++ b/packages/malloy/src/lang/malloy-to-ast.ts @@ -51,6 +51,7 @@ import { } from './parse-utils'; import {CastType} from '../model'; import { + AccessModifierLabel, DocumentLocation, DocumentRange, isCastType, @@ -59,6 +60,7 @@ import { } from '../model/malloy_types'; import {Tag} from '../tags'; import {ConstantExpression} from './ast/expressions/constant-expression'; +import {isNotUndefined} from './utils'; class ErrorNode extends ast.SourceQueryElement { elementType = 'parseErrorSourceQuery'; @@ -493,6 +495,7 @@ export class MalloyToAST } visitDefJoinMany(pcx: parse.DefJoinManyContext): ast.JoinStatement { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const joins: ast.Join[] = []; for (const joinCx of pcx.joinList().joinDef()) { const join = this.visit(joinCx); @@ -516,12 +519,13 @@ export class MalloyToAST } } } - const joinMany = new ast.JoinStatement(joins); + const joinMany = new ast.JoinStatement(joins, accessLabel); joinMany.extendNote({blockNotes: this.getNotes(pcx.tags())}); return joinMany; } visitDefJoinOne(pcx: parse.DefJoinOneContext): ast.JoinStatement { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const joinList = this.getJoinList(pcx.joinList()); const joins: ast.Join[] = []; for (const join of joinList) { @@ -532,12 +536,13 @@ export class MalloyToAST } } } - const joinOne = new ast.JoinStatement(joins); + const joinOne = new ast.JoinStatement(joins, accessLabel); joinOne.extendNote({blockNotes: this.getNotes(pcx.tags())}); return joinOne; } visitDefJoinCross(pcx: parse.DefJoinCrossContext): ast.JoinStatement { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const joinList = this.getJoinList(pcx.joinList()); const joins: ast.Join[] = []; for (const join of joinList) { @@ -553,7 +558,7 @@ export class MalloyToAST } } } - const joinCross = new ast.JoinStatement(joins); + const joinCross = new ast.JoinStatement(joins, accessLabel); joinCross.extendNote({blockNotes: this.getNotes(pcx.tags())}); return joinCross; } @@ -642,21 +647,43 @@ export class MalloyToAST } visitDefDimensions(pcx: parse.DefDimensionsContext): ast.Dimensions { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const defs = this.getFieldDefs( pcx.defList().fieldDef(), ast.DimensionFieldDeclaration ); - const stmt = new ast.Dimensions(defs); + const stmt = new ast.Dimensions(defs, accessLabel); stmt.extendNote({blockNotes: this.getNotes(pcx.tags())}); return this.astAt(stmt, pcx); } + getAccessLabel( + pcx: parse.AccessLabelContext | undefined + ): AccessModifierLabel | undefined { + if (pcx === undefined) return undefined; + if (pcx.INTERNAL_KW()) return 'internal'; + if (pcx.PRIVATE_KW()) return 'private'; + if (pcx.PUBLIC_KW()) return 'public'; + throw this.internalError(pcx, `Unknown access modifier label ${pcx.text}`); + } + + getAccessLabelProp( + pcx: parse.AccessLabelPropContext | undefined + ): AccessModifierLabel | undefined { + if (pcx === undefined) return undefined; + if (pcx.INTERNAL()) return 'internal'; + if (pcx.PRIVATE()) return 'private'; + if (pcx.PUBLIC()) return 'public'; + throw this.internalError(pcx, `Unknown access modifier label ${pcx.text}`); + } + visitDefMeasures(pcx: parse.DefMeasuresContext): ast.Measures { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const defs = this.getFieldDefs( pcx.defList().fieldDef(), ast.MeasureFieldDeclaration ); - const stmt = new ast.Measures(defs); + const stmt = new ast.Measures(defs, accessLabel); stmt.extendNote({blockNotes: this.getNotes(pcx.tags())}); return this.astAt(stmt, pcx); } @@ -682,11 +709,12 @@ export class MalloyToAST } visitDeclareStatement(pcx: parse.DeclareStatementContext): ast.DeclareFields { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const defs = this.getFieldDefs( pcx.defList().fieldDef(), ast.DeclareFieldDeclaration ); - const stmt = new ast.DeclareFields(defs); + const stmt = new ast.DeclareFields(defs, accessLabel); const result = this.astAt(stmt, pcx); this.m4advisory( pcx, @@ -707,9 +735,10 @@ export class MalloyToAST } visitDefExploreRename(pcx: parse.DefExploreRenameContext): ast.Renames { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const rcxs = pcx.renameList().exploreRenameDef(); const renames = rcxs.map(rcx => this.visitExploreRenameDef(rcx)); - const stmt = new ast.Renames(renames); + const stmt = new ast.Renames(renames, accessLabel); return this.astAt(stmt, pcx); } @@ -729,15 +758,13 @@ export class MalloyToAST return this.astAt(having, pcx); } - visitSubQueryDefList(pcx: parse.SubQueryDefListContext): ast.Views { + visitDefExploreQuery(pcx: parse.DefExploreQueryContext): ast.Views { + const accessLabel = this.getAccessLabel(pcx.accessLabel()); const babyTurtles = pcx + .subQueryDefList() .exploreQueryDef() .map(cx => this.visitExploreQueryDef(cx)); - return new ast.Views(babyTurtles); - } - - visitDefExploreQuery(pcx: parse.DefExploreQueryContext): ast.MalloyElement { - const queryDefs = this.visitSubQueryDefList(pcx.subQueryDefList()); + const queryDefs = new ast.Views(babyTurtles, accessLabel); const blockNotes = this.getNotes(pcx.tags()); queryDefs.extendNote({blockNotes}); if (pcx.QUERY()) { @@ -757,10 +784,13 @@ export class MalloyToAST return this.astAt(node, pcx); } - visitFieldNameList(pcx: parse.FieldNameListContext): ast.FieldReferences { + getFieldNameList( + pcx: parse.FieldNameListContext, + makeFieldRef: ast.FieldReferenceConstructor + ): ast.FieldReferences { const members = pcx .fieldName() - .map(cx => new ast.AcceptExceptFieldReference([this.getFieldName(cx)])); + .map(cx => this.astAt(new makeFieldRef([this.getFieldName(cx)]), cx)); return new ast.FieldReferences(members); } @@ -770,8 +800,22 @@ export class MalloyToAST const action = pcx.ACCEPT() ? 'accept' : 'except'; return new ast.FieldListEdit( action, - this.visitFieldNameList(pcx.fieldNameList()) + this.getFieldNameList(pcx.fieldNameList(), ast.AcceptExceptFieldReference) + ); + } + + visitSQInclude(pcx: parse.SQIncludeContext): ast.SQExtend { + const extendSrc = this.getSqExpr(pcx.sqExpr()); + const includeBlock = pcx.includeBlock(); + const includeList = includeBlock + ? this.getIncludeItems(includeBlock) + : undefined; + const src = new ast.SQExtend( + extendSrc, + new ast.SourceDesc([]), + includeList ); + return this.astAt(src, pcx); } visitDefExploreTimezone( @@ -999,7 +1043,7 @@ export class MalloyToAST visitCollectionWildCard( pcx: parse.CollectionWildCardContext - ): ast.FieldReferenceElement { + ): ast.WildcardFieldReference { const nameCx = pcx.fieldPath(); const join = nameCx ? this.getFieldPath(nameCx, ast.ProjectFieldReference) @@ -1832,13 +1876,125 @@ export class MalloyToAST visitSQExtendedSource(pcx: parse.SQExtendedSourceContext) { const extendSrc = this.getSqExpr(pcx.sqExpr()); + const includeBlock = pcx.includeBlock(); + const includeList = includeBlock + ? this.getIncludeItems(includeBlock) + : undefined; const src = new ast.SQExtend( extendSrc, - this.getSourceExtensions(pcx.exploreProperties()) + this.getSourceExtensions(pcx.exploreProperties()), + includeList ); return this.astAt(src, pcx); } + getIncludeItems(pcx: parse.IncludeBlockContext): ast.IncludeItem[] { + this.inExperiment('access_modifiers', pcx); + return pcx + .includeItem() + .map(item => this.getIncludeItem(item)) + .filter(isNotUndefined); + } + + getIncludeItem(pcx: parse.IncludeItemContext): ast.IncludeItem | undefined { + const tagsCx = pcx.tags(); + const blockNotes = tagsCx ? this.getNotes(tagsCx) : []; + const exceptList = pcx.includeExceptList(); + if (exceptList) { + if (tagsCx && blockNotes.length > 0) { + this.contextError( + tagsCx, + 'cannot-tag-include-except', + 'Tags on `except:` are ignored', + {severity: 'warn'} + ); + } + const fieldList = this.getExcludeList(exceptList); + return this.astAt(new ast.IncludeExceptItem(fieldList), pcx); + } else { + const listCx = pcx.includeList(); + if (listCx === undefined) { + this.contextError( + pcx.orphanedAnnotation() ?? pcx, + 'orphaned-object-annotation', + 'This tag is not attached to anything', + {severity: 'warn'} + ); + return undefined; + } + const kind = this.getAccessLabelProp(pcx.accessLabelProp()); + const fieldList = this.getIncludeList(listCx); + const item = this.astAt(new ast.IncludeAccessItem(kind, fieldList), pcx); + item.extendNote({blockNotes}); + return item; + } + } + + getIncludeList(pcx: parse.IncludeListContext): ast.IncludeListItem[] { + const listCx = pcx.includeField(); + if (listCx === undefined) { + throw this.internalError(pcx, 'Expected a field name list'); + } + return listCx.map(fieldCx => this.getIncludeListItem(fieldCx)); + } + + getExcludeList( + pcx: parse.IncludeExceptListContext + ): (ast.AccessModifierFieldReference | ast.WildcardFieldReference)[] { + return pcx.includeExceptListItem().map(fcx => { + if (fcx.tags().ANNOTATION().length > 0) { + this.contextError( + fcx.tags(), + 'cannot-tag-include-except', + 'Tags on `except:` are ignored', + {severity: 'warn'} + ); + } + const fieldNameCx = fcx.fieldName(); + if (fieldNameCx) { + return this.astAt( + new ast.AccessModifierFieldReference([ + this.astAt(new ast.FieldName(fcx.text), fcx), + ]), + fieldNameCx + ); + } + const wildcardCx = fcx.collectionWildCard(); + if (wildcardCx) { + return this.astAt(this.visitCollectionWildCard(wildcardCx), wildcardCx); + } + throw this.internalError(fcx, 'Expected a field name or wildcard'); + }); + } + + getIncludeListItem(pcx: parse.IncludeFieldContext): ast.IncludeListItem { + const wildcardCx = pcx.collectionWildCard(); + const wildcard = wildcardCx + ? this.visitCollectionWildCard(wildcardCx) + : undefined; + const as = pcx._as ? pcx._as.text : undefined; + const tags1cx = pcx.tags(); + const tags1 = tags1cx ? this.getNotes(tags1cx) : []; + const tags2cx = pcx.isDefine(); + const tags2 = tags2cx ? this.getIsNotes(tags2cx) : []; + const notes = [...tags1, ...tags2]; + const name = pcx._name + ? this.astAt( + new ast.AccessModifierFieldReference([ + this.astAt(this.getFieldName(pcx._name), pcx._name), + ]), + pcx._name + ) + : undefined; + const reference = name ?? wildcard; + if (reference === undefined) { + throw this.internalError(pcx, 'Expected a field name or wildcard'); + } + const item = this.astAt(new ast.IncludeListItem(reference, as), pcx); + item.extendNote({notes}); + return item; + } + visitSQParens(pcx: parse.SQParensContext) { // TODO maybe implement a pass-through SQParens node const sqExpr = this.getSqExpr(pcx.sqExpr()); diff --git a/packages/malloy/src/lang/parse-log.ts b/packages/malloy/src/lang/parse-log.ts index cd8caff605..a7f4a39068 100644 --- a/packages/malloy/src/lang/parse-log.ts +++ b/packages/malloy/src/lang/parse-log.ts @@ -385,7 +385,21 @@ type MessageParameterTypes = { 'or-choices-only': string; 'sql-in': string; 'dialect-cast-unsafe-only': string; + 'field-not-accessible': string; + 'cannot-expand-access': string; + 'conflicting-access-modifier': string; + 'accept-except-not-compatible-with-include': string; + 'already-renamed': string; + 'wildcard-except-redundant': string; + 'already-used-star-in-include': string; + 'include-after-exclude': string; + 'duplicate-include': string; + 'exclude-after-include': string; + 'cannot-rename-non-field': string; 'array-values-incompatible': string; + 'cannot-tag-include-except': string; + 'unsupported-path-in-include': string; + 'wildcard-include-rename': string; }; export const MESSAGE_FORMATTERS: PartialErrorCodeMessageMap = { diff --git a/packages/malloy/src/lang/test/annotation.spec.ts b/packages/malloy/src/lang/test/annotation.spec.ts index a178d7ac6d..52ca5c4c35 100644 --- a/packages/malloy/src/lang/test/annotation.spec.ts +++ b/packages/malloy/src/lang/test/annotation.spec.ts @@ -27,6 +27,7 @@ import { getFieldDef, getQueryFieldDef, model, + warningMessage, } from './test-translator'; import './parse-expects'; import {diff} from 'jest-diff'; @@ -683,4 +684,160 @@ describe('query operation annotations', () => { }); } }); + describe('include annotations', () => { + test('inherit: star', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # ai + * + } + `); + expect(m).toTranslate(); + const na = m.getSourceDef('na'); + expect(na).toBeDefined(); + if (na) { + const ai = getFieldDef(na, 'ai'); + expect(ai?.annotation).matchesAnnotation({ + blockNotes: [], + notes: ['# ai\n'], + }); + } + }); + test('new tags are inherited, not added', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # ai + ai + } include { + # ai_2 + ai + } + `); + expect(m).toTranslate(); + const na = m.getSourceDef('na'); + expect(na).toBeDefined(); + if (na) { + const ai = getFieldDef(na, 'ai'); + expect(ai?.annotation).matchesAnnotation({ + blockNotes: [], + notes: ['# ai_2\n'], + inherits: {notes: ['# ai\n'], blockNotes: []}, + }); + } + }); + test('modifier: star', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # ai_a + public: + # ai_b + * + } + `); + expect(m).toTranslate(); + const na = m.getSourceDef('na'); + expect(na).toBeDefined(); + if (na) { + const ai = getFieldDef(na, 'ai'); + expect(ai?.annotation).matchesAnnotation({ + blockNotes: ['# ai_a\n'], + notes: ['# ai_b\n'], + }); + } + }); + test('inherit: list', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # ai + ai + af + } + `); + expect(m).toTranslate(); + const na = m.getSourceDef('na'); + expect(na).toBeDefined(); + if (na) { + const ai = getFieldDef(na, 'ai'); + expect(ai?.annotation).matchesAnnotation({ + blockNotes: [], + notes: ['# ai\n'], + }); + const af = getFieldDef(na, 'af'); + expect(af).toBeDefined(); + expect(af?.annotation).toBeUndefined(); + } + }); + test('modifier: list', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # a + public: + # ai + ai + af + } + `); + expect(m).toTranslate(); + const na = m.getSourceDef('na'); + expect(na).toBeDefined(); + if (na) { + const ai = getFieldDef(na, 'ai'); + expect(ai?.annotation).matchesAnnotation({ + blockNotes: ['# a\n'], + notes: ['# ai\n'], + }); + const af = getFieldDef(na, 'af'); + expect(af?.annotation).matchesAnnotation({ + blockNotes: ['# a\n'], + notes: [], + }); + } + }); + test('tags except: list', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # error_1 + except: + # error_2 + ai + } + `); + expect(m).toLog( + warningMessage('Tags on `except:` are ignored'), + warningMessage('Tags on `except:` are ignored') + ); + }); + test('tags except: star', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + # error_1 + except: + # error_2 + * + } + `); + expect(m).toLog( + warningMessage('Tags on `except:` are ignored'), + warningMessage('Tags on `except:` are ignored'), + warningMessage('`except: *` is implied, unless another clause uses *') + ); + }); + test('oprhaned annotation', () => { + const m = new TestTranslator(` + ##! experimental.access_modifiers + source: na is a include { + ai + ${'# orphaned'} + } + `); + expect(m).toLog(warningMessage('This tag is not attached to anything')); + }); + }); }); diff --git a/packages/malloy/src/lang/test/composite-field-usage.spec.ts b/packages/malloy/src/lang/test/composite-field-usage.spec.ts index 90ff80a054..280b2ea5c9 100644 --- a/packages/malloy/src/lang/test/composite-field-usage.spec.ts +++ b/packages/malloy/src/lang/test/composite-field-usage.spec.ts @@ -191,6 +191,77 @@ describe('composite sources', () => { run: foo(param is 2) -> { group_by: y } `).toTranslate(); }); + test('composite source does not include private field', () => { + expect(` + ##! experimental { composite_sources access_modifiers } + source: foo is compose( + a extend { + private dimension: x is 1 + }, + a + ) + run: foo -> { group_by: x } + `).toLog(errorMessage("'x' is not defined")); + }); + test('composite source does not resolve to private field', () => { + expect(` + ##! experimental { composite_sources access_modifiers } + source: foo is compose( + a extend { + private dimension: x is 1 + dimension: y is 1 + }, + a extend { dimension: x is 1 } + ) + run: foo -> { group_by: x, y } + `).toLog( + errorMessage( + 'This operation uses composite field `y`, resulting in invalid usage of the composite source, as there is no composite input source which defines all of `x`, `y`' + ) + ); + }); + test('composite source does include internal field', () => { + expect(` + ##! experimental { composite_sources access_modifiers } + source: foo is compose( + a extend { + internal dimension: x is 1 + }, + a + ) extend { + view: v is { group_by: x } + } + run: foo -> v + `).toTranslate(); + }); + test('access level mismatch in composite (before)', () => { + expect(` + ##! experimental { composite_sources access_modifiers } + source: foo is compose( + a extend { + internal dimension: x is 1 + }, + a extend { + dimension: x is 1 + } + ) + run: foo -> { group_by: x } + `).toLog(errorMessage("'x' is internal")); + }); + test('access level mismatch in composite (after)', () => { + expect(` + ##! experimental { composite_sources access_modifiers } + source: foo is compose( + a extend { + dimension: x is 1 + }, + a extend { + internal dimension: x is 1 + } + ) + run: foo -> { group_by: x } + `).toLog(errorMessage("'x' is internal")); + }); test('array.each is okay', () => { expect(` ##! experimental { composite_sources } diff --git a/packages/malloy/src/lang/test/source.spec.ts b/packages/malloy/src/lang/test/source.spec.ts index 67c23837a6..7f1a941d06 100644 --- a/packages/malloy/src/lang/test/source.spec.ts +++ b/packages/malloy/src/lang/test/source.spec.ts @@ -238,6 +238,274 @@ describe('source:', () => { `).toTranslate(); }); }); + describe('access modifiers and include', () => { + test('private not accessible in query', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { *; private: ai } + run: c -> { select: ${'ai'} } + `).toLog(errorMessage("'ai' is private")); + }); + test('internal not accessible in query', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { *; internal: ai } + run: c -> { select: ${'ai'} } + `).toLog(errorMessage("'ai' is internal")); + }); + test('internal is accessible in source extension', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { *; internal: ai } + source: d is c extend { + dimension: ai2 is ai + } + `).toTranslate(); + }); + test('private is inaccessible in source extension', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: * + private: ai + } + source: d is c extend { + dimension: ai2 is ai + } + `).toLog(errorMessage("'ai' is private")); + }); + test('internal is inaccessible in joining source on', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: * + internal: ai + } + source: d is a extend { + join_one: c on ai = ${'c.ai'} + } + `).toLog(errorMessage("'ai' is internal")); + }); + test('internal at definition time', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a extend { + internal dimension: x is 1 + } + run: c -> x + `).toLog(errorMessage("'x' is internal")); + }); + test('internal is inaccessible in joining source field', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: * + internal: ai + } + source: d is a extend { + join_one: c on true + dimension: cai is ${'c.ai'} + } + `).toLog(errorMessage("'ai' is internal")); + }); + test('internal is inaccessible in view reference', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a extend { + view: v is { group_by: ai } + } include { *; internal: v } + run: c -> ${'v'} + `).toLog(errorMessage("'v' is internal")); + }); + test('private field used in view is accessible outside via view', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a extend { + view: v is { group_by: ai } + } include { *; private: ai } + run: c -> v + `).toTranslate(); + }); + test('use internal field in query in extension', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + internal: ai + } + source: d is c extend { + view: v is { group_by: ai } + } + `).toTranslate(); + }); + test('cannot expand access from private', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: * + private: ai + } + source: d is c include { + internal: ${'ai'} + } + `).toLog( + errorMessage('Cannot expand access of `ai` from private to internal') + ); + }); + test('can expand access from internal explicitly', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + internal: * + } + source: d is c include { + public: ai + } + run: d -> { group_by: ai } + `).toTranslate(); + }); + test('can expand access from internal with star', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + internal: * + } + source: d is c include { + public: * + } + run: d -> { group_by: ai } + `).toTranslate(); + }); + test('star does not expand access', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a extend { + private dimension: x is 1 + } + source: d is c include { + public: * + } + source: e is d extend { + dimension: y is ${'x'} + } + `).toLog(errorMessage("'x' is private")); + }); + test('access modifier *', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + private: * + } + run: c -> { group_by: ai } + `).toLog(errorMessage("'ai' is private")); + }); + test('private things can be used in immediate extension', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + private: * + } extend { + dimension: ai2 is ai + } + `).toTranslate(); + }); + test('private things cannot be used in later extension', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is (a include { + private: * + }) extend { + dimension: ai2 is ai + } + `).toLog(errorMessage("'ai' is private")); + }); + test('access modifier * except', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + ai; + private: * + } + run: c -> { group_by: ai, abool } + `).toLog(errorMessage("'abool' is private")); + }); + test('access modifier * nonconflicting use', () => { + expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + internal: ai + private: * + } + run: c -> { group_by: ai } + source: d is c extend { + dimension: x is abool + } + `).toLog( + errorMessage("'ai' is internal"), + errorMessage("'abool' is private") + ); + }); + test('cannot override in same source', () => { + return expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: ai + private: ${'ai'} + } + `).toLog(errorMessage('Field `ai` already referenced in include list')); + }); + test('rename in include', () => { + return expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + public: ai2 is ai + } + run: c -> { group_by: ai2 } + run: c -> { group_by: ${'ai'} } + `).toLog(errorMessage("'ai' is not defined")); + }); + test('commas optional in include', () => { + return expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + ai + astr + abool + private: + af + aweird + } + run: c -> { group_by: astr } + run: c -> { group_by: ${'af'} } + `).toLog(errorMessage("'af' is private")); + }); + test('include and except list', () => { + return expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + ai + except: astr + } + `).toLog( + errorMessage( + 'Cannot exclude specific fields if specific fields are already included' + ) + ); + }); + test('except and include list', () => { + return expect(markSource` + ##! experimental.access_modifiers + source: c is a include { + except: astr + public: ai + } + `).toLog( + errorMessage( + 'Cannot include specific fields if specific fields are already excluded' + ) + ); + }); + // TODO test conflict with `rename:` and `except:` and `accept:` + }); test('primary_key', () => { expect('source: c is a extend { primary_key: ai }').toTranslate(); }); diff --git a/packages/malloy/src/model/composite_source_utils.ts b/packages/malloy/src/model/composite_source_utils.ts index afa4a4a352..ddf1a0f75d 100644 --- a/packages/malloy/src/model/composite_source_utils.ts +++ b/packages/malloy/src/model/composite_source_utils.ts @@ -61,7 +61,9 @@ function _resolveCompositeSources( } of narrowedSources) { const fieldNames = new Set(); for (const field of inputSource.fields) { - fieldNames.add(field.as ?? field.name); + if (field.accessModifier !== 'private') { + fieldNames.add(field.as ?? field.name); + } } for (const usage of compositeFieldUsage.fields) { if (!fieldNames.has(usage)) { diff --git a/packages/malloy/src/model/malloy_types.ts b/packages/malloy/src/model/malloy_types.ts index da97f8cc90..f27b327dce 100644 --- a/packages/malloy/src/model/malloy_types.ts +++ b/packages/malloy/src/model/malloy_types.ts @@ -652,6 +652,7 @@ export function isCastType(s: string): s is CastType { export interface FieldBase extends NamedObject, Expression, ResultMetadata { annotation?: Annotation; + accessModifier?: NonDefaultAccessModifierLabel | undefined; } // this field definition represents something in the database. @@ -830,6 +831,7 @@ export interface JoinBase { matrixOperation?: MatrixOperation; onExpression?: Expr; onCompositeFieldUsage?: CompositeFieldUsage; + accessModifier?: NonDefaultAccessModifierLabel | undefined; } export type Joinable = @@ -1066,9 +1068,13 @@ export interface QuerySegment extends Filtered, Ordered { compositeFieldUsage?: CompositeFieldUsage; } +export type NonDefaultAccessModifierLabel = 'private' | 'internal'; +export type AccessModifierLabel = NonDefaultAccessModifierLabel | 'public'; + export interface TurtleDef extends NamedObject, Pipeline { type: 'turtle'; annotation?: Annotation; + accessModifier?: NonDefaultAccessModifierLabel | undefined; compositeFieldUsage?: CompositeFieldUsage; } From f989edaea027376aea458104d1ea97de77f4abfb Mon Sep 17 00:00:00 2001 From: Malloy CI Bot Date: Thu, 12 Dec 2024 22:41:50 +0000 Subject: [PATCH 3/4] Version 0.0.223-dev Signed-off-by: Malloy CI Bot --- lerna.json | 2 +- package-lock.json | 54 +++++++++---------- packages/malloy-db-bigquery/package.json | 4 +- packages/malloy-db-duckdb/package.json | 4 +- packages/malloy-db-mysql/package.json | 4 +- packages/malloy-db-postgres/package.json | 4 +- packages/malloy-db-snowflake/package.json | 4 +- packages/malloy-db-trino/package.json | 4 +- packages/malloy-interfaces/package.json | 2 +- packages/malloy-malloy-sql/package.json | 4 +- packages/malloy-render/package.json | 4 +- packages/malloy-syntax-highlight/package.json | 2 +- packages/malloy/package.json | 2 +- packages/malloy/src/version.ts | 2 +- test/package.json | 16 +++--- 15 files changed, 56 insertions(+), 56 deletions(-) diff --git a/lerna.json b/lerna.json index f93c22a579..a01aceb1a2 100644 --- a/lerna.json +++ b/lerna.json @@ -1,7 +1,7 @@ { "$schema": "node_modules/lerna/schemas/lerna-schema.json", "useNx": false, - "version": "0.0.222", + "version": "0.0.223", "command": { "version": { "allowBranch": "main" diff --git a/package-lock.json b/package-lock.json index 8db90641e6..96685eb6e4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -26598,7 +26598,7 @@ }, "packages/malloy": { "name": "@malloydata/malloy", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { "antlr4ts": "^0.5.0-alpha.4", @@ -26619,13 +26619,13 @@ }, "packages/malloy-db-bigquery": { "name": "@malloydata/db-bigquery", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { "@google-cloud/bigquery": "^7.3.0", "@google-cloud/common": "^5.0.1", "@google-cloud/paginator": "^5.0.0", - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "gaxios": "^4.2.0" }, "engines": { @@ -26634,11 +26634,11 @@ }, "packages/malloy-db-duckdb": { "name": "@malloydata/db-duckdb", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { "@duckdb/duckdb-wasm": "1.29.0", - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@motherduck/wasm-client": "^0.6.6", "apache-arrow": "^17.0.0", "duckdb": "1.1.1", @@ -26695,10 +26695,10 @@ }, "packages/malloy-db-mysql": { "name": "@malloydata/db-mysql", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@types/node": "^22.7.4", "fastestsmallesttextencoderdecoder": "^1.0.22", "luxon": "^3.5.0", @@ -26731,10 +26731,10 @@ }, "packages/malloy-db-postgres": { "name": "@malloydata/db-postgres", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@types/pg": "^8.6.1", "pg": "^8.7.1", "pg-query-stream": "4.2.3" @@ -26745,10 +26745,10 @@ }, "packages/malloy-db-snowflake": { "name": "@malloydata/db-snowflake", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "generic-pool": "^3.9.0", "snowflake-sdk": "1.14.0", "toml": "^3.0.0" @@ -26759,10 +26759,10 @@ }, "packages/malloy-db-trino": { "name": "@malloydata/db-trino", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@prestodb/presto-js-client": "^1.0.0", "gaxios": "^4.2.0", "trino-client": "^0.2.2" @@ -26773,7 +26773,7 @@ }, "packages/malloy-interfaces": { "name": "@malloydata/malloy-interfaces", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "engines": { "node": ">=18" @@ -26781,10 +26781,10 @@ }, "packages/malloy-malloy-sql": { "name": "@malloydata/malloy-sql", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222" + "@malloydata/malloy": "^0.0.223" }, "devDependencies": { "peggy": "^3.0.2" @@ -26795,10 +26795,10 @@ }, "packages/malloy-render": { "name": "@malloydata/render", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@tanstack/solid-virtual": "^3.10.4", "component-register": "^0.8.6", "lodash": "^4.17.20", @@ -27232,7 +27232,7 @@ }, "packages/malloy-syntax-highlight": { "name": "@malloydata/syntax-highlight", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "devDependencies": { "@types/jasmine": "^4.3.5", @@ -27282,17 +27282,17 @@ }, "test": { "name": "@malloydata/malloy-tests", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "dependencies": { "@jest/globals": "^29.4.3", - "@malloydata/db-bigquery": "^0.0.222", - "@malloydata/db-duckdb": "^0.0.222", - "@malloydata/db-postgres": "^0.0.222", - "@malloydata/db-snowflake": "^0.0.222", - "@malloydata/db-trino": "^0.0.222", - "@malloydata/malloy": "^0.0.222", - "@malloydata/render": "^0.0.222", + "@malloydata/db-bigquery": "^0.0.223", + "@malloydata/db-duckdb": "^0.0.223", + "@malloydata/db-postgres": "^0.0.223", + "@malloydata/db-snowflake": "^0.0.223", + "@malloydata/db-trino": "^0.0.223", + "@malloydata/malloy": "^0.0.223", + "@malloydata/render": "^0.0.223", "events": "^3.3.0", "jsdom": "^22.1.0", "luxon": "^2.4.0", diff --git a/packages/malloy-db-bigquery/package.json b/packages/malloy-db-bigquery/package.json index c147ba68f9..a92b8d1dea 100644 --- a/packages/malloy-db-bigquery/package.json +++ b/packages/malloy-db-bigquery/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-bigquery", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -25,7 +25,7 @@ "@google-cloud/bigquery": "^7.3.0", "@google-cloud/common": "^5.0.1", "@google-cloud/paginator": "^5.0.0", - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "gaxios": "^4.2.0" } } diff --git a/packages/malloy-db-duckdb/package.json b/packages/malloy-db-duckdb/package.json index 60a39e9d3f..7883dbfe0f 100644 --- a/packages/malloy-db-duckdb/package.json +++ b/packages/malloy-db-duckdb/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-duckdb", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "./dist/index.js", "types": "./dist/index.d.ts", @@ -41,7 +41,7 @@ }, "dependencies": { "@duckdb/duckdb-wasm": "1.29.0", - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@motherduck/wasm-client": "^0.6.6", "apache-arrow": "^17.0.0", "duckdb": "1.1.1", diff --git a/packages/malloy-db-mysql/package.json b/packages/malloy-db-mysql/package.json index 25b44c8047..846c7633fc 100644 --- a/packages/malloy-db-mysql/package.json +++ b/packages/malloy-db-mysql/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-mysql", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -22,7 +22,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@types/node": "^22.7.4", "fastestsmallesttextencoderdecoder": "^1.0.22", "luxon": "^3.5.0", diff --git a/packages/malloy-db-postgres/package.json b/packages/malloy-db-postgres/package.json index 1d66e194f5..3d276cc2bf 100644 --- a/packages/malloy-db-postgres/package.json +++ b/packages/malloy-db-postgres/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-postgres", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -22,7 +22,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@types/pg": "^8.6.1", "pg": "^8.7.1", "pg-query-stream": "4.2.3" diff --git a/packages/malloy-db-snowflake/package.json b/packages/malloy-db-snowflake/package.json index 83df91680e..709cf031cf 100644 --- a/packages/malloy-db-snowflake/package.json +++ b/packages/malloy-db-snowflake/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-snowflake", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -21,7 +21,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "generic-pool": "^3.9.0", "snowflake-sdk": "1.14.0", "toml": "^3.0.0" diff --git a/packages/malloy-db-trino/package.json b/packages/malloy-db-trino/package.json index cee0f19bdd..4cca7c820e 100644 --- a/packages/malloy-db-trino/package.json +++ b/packages/malloy-db-trino/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/db-trino", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -22,7 +22,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@prestodb/presto-js-client": "^1.0.0", "gaxios": "^4.2.0", "trino-client": "^0.2.2" diff --git a/packages/malloy-interfaces/package.json b/packages/malloy-interfaces/package.json index fab4a7bff9..30e582e541 100644 --- a/packages/malloy-interfaces/package.json +++ b/packages/malloy-interfaces/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/malloy-interfaces", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", diff --git a/packages/malloy-malloy-sql/package.json b/packages/malloy-malloy-sql/package.json index e8eaab8581..99b26cb5e4 100644 --- a/packages/malloy-malloy-sql/package.json +++ b/packages/malloy-malloy-sql/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/malloy-sql", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/index.js", "types": "dist/index.d.ts", @@ -24,7 +24,7 @@ "prepublishOnly": "npm run build" }, "dependencies": { - "@malloydata/malloy": "^0.0.222" + "@malloydata/malloy": "^0.0.223" }, "devDependencies": { "peggy": "^3.0.2" diff --git a/packages/malloy-render/package.json b/packages/malloy-render/package.json index 84f4a30cb1..efbd564fc5 100644 --- a/packages/malloy-render/package.json +++ b/packages/malloy-render/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/render", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "main": "dist/module/index.umd.js", "types": "dist/index.d.ts", @@ -30,7 +30,7 @@ "build-types": "tsc --build --declaration --emitDeclarationOnly" }, "dependencies": { - "@malloydata/malloy": "^0.0.222", + "@malloydata/malloy": "^0.0.223", "@tanstack/solid-virtual": "^3.10.4", "component-register": "^0.8.6", "lodash": "^4.17.20", diff --git a/packages/malloy-syntax-highlight/package.json b/packages/malloy-syntax-highlight/package.json index e1a4175b0f..772fe7216e 100644 --- a/packages/malloy-syntax-highlight/package.json +++ b/packages/malloy-syntax-highlight/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/syntax-highlight", - "version": "0.0.222", + "version": "0.0.223", "description": "A package to simplify the process of developing, testing, and syncnig Malloy syntax highlighting grammars", "files": [ "grammars/**/*.tmGrammar.json", diff --git a/packages/malloy/package.json b/packages/malloy/package.json index a445786779..6d4910dcda 100644 --- a/packages/malloy/package.json +++ b/packages/malloy/package.json @@ -1,6 +1,6 @@ { "name": "@malloydata/malloy", - "version": "0.0.222", + "version": "0.0.223", "license": "MIT", "exports": { ".": "./dist/index.js", diff --git a/packages/malloy/src/version.ts b/packages/malloy/src/version.ts index e1fa740715..7ab6910257 100644 --- a/packages/malloy/src/version.ts +++ b/packages/malloy/src/version.ts @@ -1,2 +1,2 @@ // generated with 'generate-version-file' script; do not edit manually -export const MALLOY_VERSION = '0.0.222'; +export const MALLOY_VERSION = '0.0.223'; diff --git a/test/package.json b/test/package.json index ef1adc9d57..91570a8f36 100644 --- a/test/package.json +++ b/test/package.json @@ -21,13 +21,13 @@ }, "dependencies": { "@jest/globals": "^29.4.3", - "@malloydata/db-bigquery": "^0.0.222", - "@malloydata/db-duckdb": "^0.0.222", - "@malloydata/db-postgres": "^0.0.222", - "@malloydata/db-snowflake": "^0.0.222", - "@malloydata/db-trino": "^0.0.222", - "@malloydata/malloy": "^0.0.222", - "@malloydata/render": "^0.0.222", + "@malloydata/db-bigquery": "^0.0.223", + "@malloydata/db-duckdb": "^0.0.223", + "@malloydata/db-postgres": "^0.0.223", + "@malloydata/db-snowflake": "^0.0.223", + "@malloydata/db-trino": "^0.0.223", + "@malloydata/malloy": "^0.0.223", + "@malloydata/render": "^0.0.223", "events": "^3.3.0", "jsdom": "^22.1.0", "luxon": "^2.4.0", @@ -37,5 +37,5 @@ "@types/jsdom": "^21.1.1", "@types/luxon": "^2.4.0" }, - "version": "0.0.222" + "version": "0.0.223" } From cdd3586c28054e56dab29e475da6b24cb4122987 Mon Sep 17 00:00:00 2001 From: Michael Toy <66150587+mtoy-googly-moogly@users.noreply.github.com> Date: Thu, 12 Dec 2024 20:17:22 -0800 Subject: [PATCH 4/4] allow implies keys in arrys of records (#2048) --- .../ast/expressions/expr-array-literal.ts | 6 ++- .../ast/expressions/expr-record-literal.ts | 52 ++++++++++++++++--- .../malloy/src/lang/grammar/MalloyParser.g4 | 4 +- packages/malloy/src/lang/malloy-to-ast.ts | 24 +++------ packages/malloy/src/lang/parse-log.ts | 1 + .../malloy/src/lang/test/literals.spec.ts | 18 +++++++ .../malloy/src/lang/test/parse-expects.ts | 11 ++++ 7 files changed, 89 insertions(+), 27 deletions(-) diff --git a/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts b/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts index a7e9836a22..22cf649cdd 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-array-literal.ts @@ -10,6 +10,7 @@ import {ExprValue, computedExprValue} from '../types/expr-value'; import {ExpressionDef} from '../types/expression-def'; import {FieldSpace} from '../types/field-space'; import * as TDU from '../typedesc-utils'; +import {RecordLiteral} from './expr-record-literal'; export class ArrayLiteral extends ExpressionDef { elementType = 'array literal'; @@ -24,7 +25,10 @@ export class ArrayLiteral extends ExpressionDef { let firstValue: ExprValue | undefined = undefined; if (this.elements.length > 0) { for (const nextElement of this.elements) { - const v = nextElement.getExpression(fs); + const v = + firstValue && nextElement instanceof RecordLiteral + ? nextElement.getNextElement(fs, firstValue) + : nextElement.getExpression(fs); fromValues.push(v); if (v.type === 'error') { continue; diff --git a/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts b/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts index 5ea1884101..480d0f197e 100644 --- a/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts +++ b/packages/malloy/src/lang/ast/expressions/expr-record-literal.ts @@ -11,15 +11,29 @@ import {ExpressionDef} from '../types/expression-def'; import {FieldSpace} from '../types/field-space'; import {MalloyElement} from '../types/malloy-element'; import * as TDU from '../typedesc-utils'; +import {ExprIdReference} from './expr-id-reference'; +export type ElementDetails = + | {path: ExprIdReference} + | {key?: string; value: ExpressionDef}; export class RecordElement extends MalloyElement { elementType = 'record element'; - constructor( - readonly key: string, - readonly value: ExpressionDef - ) { + value: ExpressionDef; + key?: string; + constructor(val: ElementDetails) { super(); - this.has({value}); + if ('value' in val) { + this.value = val.value; + this.has({value: val.value}); + if (val.key) { + this.key = val.key; + } + } else { + this.has({path: val.path}); + this.value = val.path; + const parts = val.path.fieldReference.path; + this.key = parts[parts.length - 1]; + } } } @@ -31,6 +45,10 @@ export class RecordLiteral extends ExpressionDef { } getExpression(fs: FieldSpace): ExprValue { + return this.getRecord(fs, []); + } + + getRecord(fs: FieldSpace, kidNames: string[]): ExprValue { const recLit: RecordLiteralNode = { node: 'recordLiteral', kids: {}, @@ -40,14 +58,24 @@ export class RecordLiteral extends ExpressionDef { }, }; const dependents: ExprValue[] = []; + let kidIndex = 0; for (const el of this.pairs) { + const key = el.key ?? kidNames[kidIndex]; + kidIndex += 1; + if (key === undefined) { + el.logError( + 'record-literal-needs-keys', + 'Anonymous record element not legal here' + ); + continue; + } const xVal = el.value.getExpression(fs); if (TD.isAtomic(xVal)) { dependents.push(xVal); - recLit.kids[el.key] = xVal.value; - recLit.typeDef.fields.push(mkFieldDef(TDU.atomicDef(xVal), el.key)); + recLit.kids[key] = xVal.value; + recLit.typeDef.fields.push(mkFieldDef(TDU.atomicDef(xVal), key)); } else { - this.logError( + el.value.logError( 'illegal-record-property-type', `Record property '${el.key} is type '${xVal.type}', which is not a legal property value type` ); @@ -59,4 +87,12 @@ export class RecordLiteral extends ExpressionDef { from: dependents, }); } + + getNextElement(fs: FieldSpace, headValue: ExprValue): ExprValue { + const recLit = headValue.value; + if (recLit.node === 'recordLiteral') { + return this.getRecord(fs, Object.keys(recLit.kids)); + } + return this.getRecord(fs, []); + } } diff --git a/packages/malloy/src/lang/grammar/MalloyParser.g4 b/packages/malloy/src/lang/grammar/MalloyParser.g4 index f9f138f5ba..c19a38eba7 100644 --- a/packages/malloy/src/lang/grammar/MalloyParser.g4 +++ b/packages/malloy/src/lang/grammar/MalloyParser.g4 @@ -664,8 +664,8 @@ caseWhen recordKey: id; recordElement - : fieldPath # recordRef - | recordKey IS fieldExpr # recordExpr + : fieldPath # recordRef + | (recordKey IS)? fieldExpr # recordExpr ; argumentList diff --git a/packages/malloy/src/lang/malloy-to-ast.ts b/packages/malloy/src/lang/malloy-to-ast.ts index 883f2d8487..5accfd87f6 100644 --- a/packages/malloy/src/lang/malloy-to-ast.ts +++ b/packages/malloy/src/lang/malloy-to-ast.ts @@ -2083,32 +2083,24 @@ export class MalloyToAST } visitRecordRef(pcx: parse.RecordRefContext) { - const pathCx = pcx.fieldPath(); - const tailEl = pathCx.fieldName().at(-1); - if (tailEl) { - const elementKey = getId(tailEl); - const idRef = new ast.ExprIdReference( - this.getFieldPath(pathCx, ast.ExpressionFieldReference) - ); - return new ast.RecordElement(elementKey, idRef); - } - throw this.internalError( - pathCx, - 'IMPOSSIBLY A PATH CONTAINED ZERO ELEMENTS' + const idRef = new ast.ExprIdReference( + this.getFieldPath(pcx.fieldPath(), ast.ExpressionFieldReference) ); + return this.astAt(new ast.RecordElement({path: idRef}), pcx); } visitRecordExpr(pcx: parse.RecordExprContext) { - const elementKey = getId(pcx.recordKey()); - const elementVal = this.getFieldExpr(pcx.fieldExpr()); - return new ast.RecordElement(elementKey, elementVal); + const value = this.getFieldExpr(pcx.fieldExpr()); + const keyCx = pcx.recordKey(); + const recInit = keyCx ? {key: getId(keyCx), value} : {value}; + return this.astAt(new ast.RecordElement(recInit), pcx); } visitExprLiteralRecord(pcx: parse.ExprLiteralRecordContext) { const els = this.only( pcx.recordElement().map(elCx => this.astAt(this.visit(elCx), elCx)), visited => visited instanceof ast.RecordElement && visited, - 'a key value pair' + 'a legal record property description' ); return new ast.RecordLiteral(els); } diff --git a/packages/malloy/src/lang/parse-log.ts b/packages/malloy/src/lang/parse-log.ts index a7f4a39068..cfd0a10f74 100644 --- a/packages/malloy/src/lang/parse-log.ts +++ b/packages/malloy/src/lang/parse-log.ts @@ -367,6 +367,7 @@ type MessageParameterTypes = { 'sql-is-not-null': string; 'sql-is-null': string; 'illegal-record-property-type': string; + 'record-literal-needs-keys': string; 'not-yet-implemented': string; 'sql-case': string; 'case-then-type-does-not-match': { diff --git a/packages/malloy/src/lang/test/literals.spec.ts b/packages/malloy/src/lang/test/literals.spec.ts index 414d4230b0..e0a1406ebc 100644 --- a/packages/malloy/src/lang/test/literals.spec.ts +++ b/packages/malloy/src/lang/test/literals.spec.ts @@ -283,4 +283,22 @@ describe('literals', () => { }); test('a string containing a tab', () => expect(expr`'\t'`).toParse()); }); + describe('compound literals', () => { + test('simple record literal', () => { + expect('{answer is 42}').compilesTo('{answer:42}'); + }); + test('record literal with path', () => { + expect('{aninline.column}').compilesTo('{column:aninline.column}'); + }); + test('array of records with same schema', () => { + expect( + '[{name is "one", val is 1},{name is "two", val is 2}]' + ).compilesTo('[{name:"one", val:1}, {name:"two", val:2}]'); + }); + test('array of records with head schema', () => { + expect('[{name is "one", val is 1},{"two", 2}]').compilesTo( + '[{name:"one", val:1}, {name:"two", val:2}]' + ); + }); + }); }); diff --git a/packages/malloy/src/lang/test/parse-expects.ts b/packages/malloy/src/lang/test/parse-expects.ts index 5b49130f8c..7aff60d078 100644 --- a/packages/malloy/src/lang/test/parse-expects.ts +++ b/packages/malloy/src/lang/test/parse-expects.ts @@ -229,6 +229,17 @@ function eToStr(e: Expr, symbols: ESymbols): string { return `"${e.literal}"`; case 'timeLiteral': return `@${e.literal}`; + case 'recordLiteral': { + const parts: string[] = []; + for (const [name, val] of Object.entries(e.kids)) { + parts.push(`${name}:${subExpr(val)}`); + } + return `{${parts.join(', ')}}`; + } + case 'arrayLiteral': { + const parts = e.kids.values.map(k => subExpr(k)); + return `[${parts.join(', ')}]`; + } case 'regexpLiteral': return `/${e.literal}/`; case 'trunc':