From 374efd202177068b10933411b1311c56fb0ac3ce Mon Sep 17 00:00:00 2001 From: Jan Melcher Date: Thu, 12 Sep 2024 17:41:31 +0200 Subject: [PATCH] refactor: use single "&" instead of "&&" to separate and-combined modules There is no need to distinguish between & and &&, so it does not make sense to have && in the first place. GraphQL uses a single &, too. While some languages uses && but do not have &, they usually use it for a conditional, but module combination is not really a conditional expression. --- .../effective-module-specification.spec.ts | 6 ++--- .../project/select-modules-in-sources.spec.ts | 2 +- .../modules-validator.spec.ts | 26 +++++++++---------- .../modules/effective-module-specification.ts | 2 +- .../modules/expression-parser.ts | 24 +++++------------ src/schema/graphql-base.ts | 2 +- 6 files changed, 25 insertions(+), 37 deletions(-) diff --git a/spec/model/implementation/effective-module-specification.spec.ts b/spec/model/implementation/effective-module-specification.spec.ts index 220499ca..8f6706e1 100644 --- a/spec/model/implementation/effective-module-specification.spec.ts +++ b/spec/model/implementation/effective-module-specification.spec.ts @@ -27,7 +27,7 @@ describe('EffectiveModuleSpecification', () => { ], }); const output = input.simplify(); - expect(output.toString()).to.equal('b, a && c'); + expect(output.toString()).to.equal('b, a & c'); }); it('removes redundant clauses', () => { @@ -39,7 +39,7 @@ describe('EffectiveModuleSpecification', () => { ], }); const output = input.simplify(); - expect(output.toString()).to.equal('b, a && c'); + expect(output.toString()).to.equal('b, a & c'); }); it('works with a more complicated case', () => { @@ -64,7 +64,7 @@ describe('EffectiveModuleSpecification', () => { ], }); const output = input.simplify(); - expect(output.toString()).to.equal('extra1, shipping, dangerous_goods && tms'); + expect(output.toString()).to.equal('extra1, shipping, dangerous_goods & tms'); }); }); }); diff --git a/spec/project/select-modules-in-sources.spec.ts b/spec/project/select-modules-in-sources.spec.ts index 2c6be8ae..96b9508d 100644 --- a/spec/project/select-modules-in-sources.spec.ts +++ b/spec/project/select-modules-in-sources.spec.ts @@ -13,7 +13,7 @@ describe('selectModulesInProjectSource', () => { type OneAndTwo @rootEntity @modules(in: ["module1", "module2"]) { all: String @modules(all: true) two: String @modules(in: "module2") - extra1: String @modules(in: ["extra1", "module2 && extra2"]) + extra1: String @modules(in: ["extra1", "module2 & extra2"]) extra2: String @modules(in: ["extra2"]) one: String @modules(in: "module1") } diff --git a/spec/schema/ast-validation-modules/modules-validator.spec.ts b/spec/schema/ast-validation-modules/modules-validator.spec.ts index 52b7376e..0025b781 100644 --- a/spec/schema/ast-validation-modules/modules-validator.spec.ts +++ b/spec/schema/ast-validation-modules/modules-validator.spec.ts @@ -330,7 +330,7 @@ describe('modules validator', () => { it('accepts an and combination of two modules', () => { assertValidatorAcceptsAndDoesNotWarn( ` - type Foo @rootEntity @modules(in: ["module1 && module2"]) { + type Foo @rootEntity @modules(in: ["module1 & module2"]) { foo: String @modules(all: true) } `, @@ -341,7 +341,7 @@ describe('modules validator', () => { it('accepts an and combination of two modules without space', () => { assertValidatorAcceptsAndDoesNotWarn( ` - type Foo @rootEntity @modules(in: ["module1&&module2"]) { + type Foo @rootEntity @modules(in: ["module1&module2"]) { foo: String @modules(all: true) } `, @@ -352,7 +352,7 @@ describe('modules validator', () => { it('accepts an and combination of three modules', () => { assertValidatorAcceptsAndDoesNotWarn( ` - type Foo @rootEntity @modules(in: ["module1 && module2 && module3"]) { + type Foo @rootEntity @modules(in: ["module1 & module2 & module3"]) { foo: String @modules(all: true) } `, @@ -372,14 +372,14 @@ describe('modules validator', () => { ); }); - it('rejects an expression with a singular &', () => { + it('rejects an expression with a double &&', () => { assertValidatorRejects( ` - type Foo @rootEntity @modules(in: ["module1 & module2"]) { + type Foo @rootEntity @modules(in: ["module1 && module2"]) { foo: String @modules(all: true) } `, - 'Expected "&&", but only got single "&".', + 'Expected identifier, but got "&".', { withModuleDefinitions: true }, ); }); @@ -391,15 +391,15 @@ describe('modules validator', () => { foo: String @modules(all: true) } `, - 'Expected "&&", but got "m".', + 'Expected "&", but got "m".', { withModuleDefinitions: true }, ); }); - it('rejects an expression that ends with &&', () => { + it('rejects an expression that ends with &', () => { assertValidatorRejects( ` - type Foo @rootEntity @modules(in: ["module1 &&"]) { + type Foo @rootEntity @modules(in: ["module1 &"]) { foo: String @modules(all: true) } `, @@ -415,15 +415,15 @@ describe('modules validator', () => { foo: String @modules(all: true) } `, - 'Expected "&&", but only got single "&".', + 'Expected identifier.', { withModuleDefinitions: true }, ); }); - it('rejects an expression that starts with &&', () => { + it('rejects an expression that starts with &', () => { assertValidatorRejects( ` - type Foo @rootEntity @modules(in: ["&& module1"]) { + type Foo @rootEntity @modules(in: ["& module1"]) { foo: String @modules(all: true) } `, @@ -439,7 +439,7 @@ describe('modules validator', () => { foo: String @modules(all: true) } `, - 'Expected identifier or "&&", but got "!".', + 'Expected identifier or "&", but got "!".', { withModuleDefinitions: true }, ); }); diff --git a/src/model/implementation/modules/effective-module-specification.ts b/src/model/implementation/modules/effective-module-specification.ts index e3b42fa0..6518b6ca 100644 --- a/src/model/implementation/modules/effective-module-specification.ts +++ b/src/model/implementation/modules/effective-module-specification.ts @@ -126,6 +126,6 @@ export class EffectiveModuleSpecificationClause { } toString() { - return this.andCombinedModules.join(' && '); + return this.andCombinedModules.join(' & '); } } diff --git a/src/model/implementation/modules/expression-parser.ts b/src/model/implementation/modules/expression-parser.ts index 3ec2fbfc..5f70315d 100644 --- a/src/model/implementation/modules/expression-parser.ts +++ b/src/model/implementation/modules/expression-parser.ts @@ -4,7 +4,6 @@ enum ParserState { EXPECT_IDENTIFIER, PARSING_IDENTIFIER, EXPECT_AMPERSAND_OR_END, - EXPECT_SECOND_AMPERSAND, } const EOF = Symbol('EOF'); @@ -70,7 +69,8 @@ export function parseModuleSpecificationExpression( offset: offset - currentIdentifer.length, }); if (char === '&') { - state = ParserState.EXPECT_SECOND_AMPERSAND; + // expecting next module + state = ParserState.EXPECT_IDENTIFIER; } else if (isWhitespace) { state = ParserState.EXPECT_AMPERSAND_OR_END; } else if (isEOF) { @@ -79,36 +79,24 @@ export function parseModuleSpecificationExpression( return { error: { offset, - message: `Expected identifier or "&&", but got "${char}".`, + message: `Expected identifier or "&", but got "${char}".`, }, }; } } break; - case ParserState.EXPECT_SECOND_AMPERSAND: - if (char === '&') { - state = ParserState.EXPECT_IDENTIFIER; - } else { - // user probably didn't know that you need to double the & - return { - error: { - offset: offset - 1, - message: `Expected "&&", but only got single "&".`, - }, - }; - } - break; case ParserState.EXPECT_AMPERSAND_OR_END: if (char === '&') { - state = ParserState.EXPECT_SECOND_AMPERSAND; + // expecting next module + state = ParserState.EXPECT_IDENTIFIER; } else if (isEOF) { // do nothing } else { return { error: { offset, - message: `Expected "&&", but got "${char}".`, + message: `Expected "&", but got "${char}".`, }, }; } diff --git a/src/schema/graphql-base.ts b/src/schema/graphql-base.ts index f2a88ef3..f03e8640 100644 --- a/src/schema/graphql-base.ts +++ b/src/schema/graphql-base.ts @@ -267,7 +267,7 @@ const directivesBase: DocumentNode = gql` """ A list of modules this type or field should be part of. - Can be an expression like module1 && module2. + Can be an expression like module1 & module2. Can include modules that are not listed in the declaring type. """