Skip to content

Commit

Permalink
refactor: use single "&" instead of "&&" to separate and-combined mod…
Browse files Browse the repository at this point in the history 
…ules

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.
  • Loading branch information
@Yogu
Yogu committed Sep 13, 2024
1 parent f066a10 commit 374efd2
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 37 deletions.
6 changes: 3 additions & 3 deletions spec/model/implementation/effective-module-specification.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand All @@ -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');
});
});
});
2 changes: 1 addition & 1 deletion spec/project/select-modules-in-sources.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
26 changes: 13 additions & 13 deletions spec/schema/ast-validation-modules/modules-validator.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
`,
Expand All @@ -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)
}
`,
Expand All @@ -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)
}
`,
Expand All @@ -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 },
);
});
Expand All @@ -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)
}
`,
Expand All @@ -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)
}
`,
Expand All @@ -439,7 +439,7 @@ describe('modules validator', () => {
foo: String @modules(all: true)
}
`,
'Expected identifier or "&&", but got "!".',
'Expected identifier or "&", but got "!".',
{ withModuleDefinitions: true },
);
});
Expand Down
2 changes: 1 addition & 1 deletion src/model/implementation/modules/effective-module-specification.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,6 @@ export class EffectiveModuleSpecificationClause {
}

toString() {
return this.andCombinedModules.join(' && ');
return this.andCombinedModules.join(' & ');
}
}
24 changes: 6 additions & 18 deletions src/model/implementation/modules/expression-parser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ enum ParserState {
EXPECT_IDENTIFIER,
PARSING_IDENTIFIER,
EXPECT_AMPERSAND_OR_END,
EXPECT_SECOND_AMPERSAND,
}

const EOF = Symbol('EOF');
Expand Down Expand Up @@ -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) {
Expand All @@ -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}".`,
},
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/schema/graphql-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
"""
Expand Down

0 comments on commit 374efd2

Please sign in to comment.