diff --git a/spec/schema/ast-validation-modules/suppress.spec.ts b/spec/schema/ast-validation-modules/suppress.spec.ts new file mode 100644 index 00000000..a3481e63 --- /dev/null +++ b/spec/schema/ast-validation-modules/suppress.spec.ts @@ -0,0 +1,93 @@ +import { assertValidatorAcceptsAndDoesNotWarn, assertValidatorWarns } from './helpers'; + +describe('@suppress', () => { + it('warns on type level if @suppress is not used', () => { + assertValidatorWarns( + ` + type Stuff @rootEntity { + foo: String + } + type Child @childEntity { + stuff: Int + } + `, + 'Type "Child" is not used.', + ); + }); + + it('does not warn on type level if @suppress is used correctly with one entry', () => { + assertValidatorAcceptsAndDoesNotWarn( + ` + type Stuff @rootEntity { + foo: String + } + type Child @childEntity @suppress(warnings: [UNUSED]) { + stuff: Int + } + `, + ); + }); + + it('does not warn on type level if @suppress is used correctly with one non-list entry', () => { + assertValidatorAcceptsAndDoesNotWarn( + ` + type Stuff @rootEntity { + foo: String + } + type Child @childEntity @suppress(warnings: UNUSED) { + stuff: Int + } + `, + ); + }); + + it('does not warn on type level if @suppress is used correctly with multiple entries', () => { + assertValidatorAcceptsAndDoesNotWarn( + ` + type Stuff @rootEntity { + foo: String + } + type Child @childEntity @suppress(warnings: [DEPRECATED, UNUSED]) { + stuff: Int + } + `, + ); + }); + + it('warns on type level if @suppress is used with the wrong code entries', () => { + assertValidatorWarns( + ` + type Stuff @rootEntity { + foo: String + } + type Child @childEntity @suppress(warnings: [DEPRECATED]) { + stuff: Int + } + `, + 'Type "Child" is not used.', + ); + }); + + it('warns on field level if @suppress is not used', () => { + assertValidatorWarns( + ` + type Stuff @rootEntity { + foo: String + _key: ID @key + } + `, + 'The field "_key" is deprecated and should be replaced with "id" (of type "ID").', + ); + }); + + it('does not warn on field level if @suppress is used', () => { + assertValidatorAcceptsAndDoesNotWarn( + ` + type Stuff @rootEntity { + foo: String + _key: ID @key @suppress(warnings: [DEPRECATED]) + } + `, + ); + }); +}); diff --git a/src/model/compatibility-check/check-calc-mutations.ts b/src/model/compatibility-check/check-calc-mutations.ts index 0e7f82fa..5d3106bb 100644 --- a/src/model/compatibility-check/check-calc-mutations.ts +++ b/src/model/compatibility-check/check-calc-mutations.ts @@ -22,7 +22,8 @@ export function checkCalcMutations( const operatorsDesc = operators.map((o) => o.toString()).join(', '); const expectedDeclaration = `@calcMutations(operators: [${operatorsDesc}])`; context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'CALC_MUTATIONS', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with ${expectedDeclaration}${getRequiredBySuffix( @@ -53,9 +54,13 @@ export function checkCalcMutations( (a) => a.name.value === CALC_MUTATIONS_OPERATORS_ARG, ); context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'CALC_MUTATIONS', `${message}${getRequiredBySuffix(baselineField)}.`, - operatorsAstNode ?? fieldToCheck.calcMutationsAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { + location: operatorsAstNode ?? fieldToCheck.calcMutationsAstNode, + }, ), ); return; diff --git a/src/model/compatibility-check/check-collect-field.ts b/src/model/compatibility-check/check-collect-field.ts index cbd66801..75b1195c 100644 --- a/src/model/compatibility-check/check-collect-field.ts +++ b/src/model/compatibility-check/check-collect-field.ts @@ -5,15 +5,21 @@ import { getRequiredBySuffix } from './describe-module-specification'; /** * Checks whether the @collect directives on the field and on the baseline fields match */ -export function checkCollectField(fieldToCheck: Field, baselineField: Field, context: ValidationContext) { +export function checkCollectField( + fieldToCheck: Field, + baselineField: Field, + context: ValidationContext, +) { // superfluous @collect if (fieldToCheck.isCollectField && !baselineField.isCollectField) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'COLLECT', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be a collect field${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.collectAstNode, + fieldToCheck.astNode, + { location: fieldToCheck.collectAstNode }, ), ); return; @@ -35,7 +41,8 @@ export function checkCollectField(fieldToCheck: Field, baselineField: Field, con expectedAggregateDeclaration ? ', ' + expectedAggregateDeclaration : '' })`; context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'COLLECT', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with ${expectedCollectDeclaration}${getRequiredBySuffix( @@ -54,13 +61,13 @@ export function checkCollectField(fieldToCheck: Field, baselineField: Field, con fieldToCheck.collectPath.path !== baselineField.collectPath.path ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'COLLECT', `Path should be "${baselineField.collectPath.path}"${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.collectPathAstNode ?? - fieldToCheck.collectAstNode ?? - fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.collectPathAstNode ?? fieldToCheck.collectAstNode }, ), ); return; @@ -69,9 +76,11 @@ export function checkCollectField(fieldToCheck: Field, baselineField: Field, con // superfluous aggregate if (fieldToCheck.aggregationOperator && !baselineField.aggregationOperator) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'COLLECT', `No aggregation should be used here${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.aggregationOperatorAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.aggregationOperatorAstNode }, ), ); } @@ -82,13 +91,13 @@ export function checkCollectField(fieldToCheck: Field, baselineField: Field, con fieldToCheck.aggregationOperator !== baselineField.aggregationOperator ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'COLLECT', `Collect field should specify ${expectedAggregateDeclaration}${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.inverseOfAstNode ?? - fieldToCheck.relationAstNode ?? - fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.aggregationOperatorAstNode }, ), ); } diff --git a/src/model/compatibility-check/check-default-value.ts b/src/model/compatibility-check/check-default-value.ts index c92b0af1..e536362f 100644 --- a/src/model/compatibility-check/check-default-value.ts +++ b/src/model/compatibility-check/check-default-value.ts @@ -20,11 +20,13 @@ export function checkDefaultValue( !baselineField.hasDefaultValue ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'DEFAULT_VALUE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not have a default value${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.defaultValueAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.defaultValueAstNode }, ), ); return; @@ -41,13 +43,15 @@ export function checkDefaultValue( if (!fieldToCheck.hasDefaultValue) { const expectedDeclaration = `@defaultValue(value: ${expectedDefaultValue})`; context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'DEFAULT_VALUE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with ${expectedDeclaration}${getRequiredBySuffix( baselineField, )}.`, fieldToCheck.astNode, + { location: fieldToCheck.defaultValueAstNode }, ), ); return; @@ -56,11 +60,13 @@ export function checkDefaultValue( // wrong default value if (!deepEqual(fieldToCheck.defaultValue, baselineField.defaultValue)) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'DEFAULT_VALUE', `Default value should be ${expectedDefaultValue}${getRequiredBySuffix( baselineField, )}.`, fieldToCheck.astNode, + { location: fieldToCheck.defaultValueAstNode }, ), ); return; diff --git a/src/model/compatibility-check/check-enum-type.ts b/src/model/compatibility-check/check-enum-type.ts index e80e196a..3dca771b 100644 --- a/src/model/compatibility-check/check-enum-type.ts +++ b/src/model/compatibility-check/check-enum-type.ts @@ -11,11 +11,13 @@ export function checkEnumType( const matchingValue = typeToCheck.values.find((v) => v.value === baselineValue.value); if (!matchingValue) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'MISSING_ENUM_VALUE', `Enum value "${baselineValue.value}" is missing${getRequiredBySuffix( baselineType, )}.`, - typeToCheck.nameASTNode, + typeToCheck.astNode, + { location: typeToCheck.nameASTNode }, ), ); } diff --git a/src/model/compatibility-check/check-field-type.ts b/src/model/compatibility-check/check-field-type.ts index fc4e5f94..a4417c3d 100644 --- a/src/model/compatibility-check/check-field-type.ts +++ b/src/model/compatibility-check/check-field-type.ts @@ -31,32 +31,44 @@ export function checkFieldType( if (fieldToCheck.type.name !== baselineField.type.name) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'FIELD_TYPE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" needs to be of type "${expectedType}"${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.astNode?.type, - { quickFixes }, + fieldToCheck.astNode, + { + location: fieldToCheck.astNode?.type, + quickFixes, + }, ), ); } else if (fieldToCheck.isList && !baselineField.isList) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'FIELD_TYPE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be a list${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.astNode?.type, - { quickFixes }, + fieldToCheck.astNode, + { + location: fieldToCheck.astNode?.type, + quickFixes, + }, ), ); } else if (!fieldToCheck.isList && baselineField.isList) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'FIELD_TYPE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" needs to be a list${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.astNode?.type, - { quickFixes }, + fieldToCheck.astNode, + { + location: fieldToCheck.astNode?.type, + quickFixes, + }, ), ); } diff --git a/src/model/compatibility-check/check-key-field.ts b/src/model/compatibility-check/check-key-field.ts index f43fb32a..003d9a62 100644 --- a/src/model/compatibility-check/check-key-field.ts +++ b/src/model/compatibility-check/check-key-field.ts @@ -23,16 +23,19 @@ export function checkKeyField( // (because it's an automatic system field). We need to tell the user that the field needs to be added. if (fieldToCheck.isSystemField && fieldToCheck.name === ID_FIELD && !fieldToCheck.astNode) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'MISSING_FIELD', `Field "id: ID @key" needs to be specified${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.declaringType.nameASTNode, + fieldToCheck.declaringType.astNode, + { location: fieldToCheck.declaringType.nameASTNode }, ), ); } else { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'KEY_FIELD', `Field "${baselineField.declaringType.name}.${ baselineField.name }" needs to be decorated with @key${getRequiredBySuffix(baselineField)}.`, diff --git a/src/model/compatibility-check/check-model.ts b/src/model/compatibility-check/check-model.ts index da4c436c..be8cc5ca 100644 --- a/src/model/compatibility-check/check-model.ts +++ b/src/model/compatibility-check/check-model.ts @@ -51,7 +51,7 @@ export function checkModel(modelToCheck: Model, baselineModel: Model): Validatio } context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.nonSuppressableCompatibilityIssue( `Type "${baselineType.name}" is missing${getRequiredBySuffix(baselineType)}.`, undefined, { quickFixes }, diff --git a/src/model/compatibility-check/check-object-type.ts b/src/model/compatibility-check/check-object-type.ts index 0b82ad8a..0965538e 100644 --- a/src/model/compatibility-check/check-object-type.ts +++ b/src/model/compatibility-check/check-object-type.ts @@ -45,12 +45,13 @@ export function checkObjectType( } context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'MISSING_FIELD', `Field "${baselineType.name}.${ baselineField.name }" is missing${getRequiredBySuffix(baselineField)}.`, - typeToCheck.nameASTNode, - { quickFixes }, + typeToCheck.astNode, + { location: typeToCheck.nameASTNode, quickFixes }, ), ); continue; diff --git a/src/model/compatibility-check/check-reference.ts b/src/model/compatibility-check/check-reference.ts index db2db545..06e1f374 100644 --- a/src/model/compatibility-check/check-reference.ts +++ b/src/model/compatibility-check/check-reference.ts @@ -5,14 +5,22 @@ import { getRequiredBySuffix } from './describe-module-specification'; /** * Checks whether the @reference directives on the field and on the baseline field match */ -export function checkReference(fieldToCheck: Field, baselineField: Field, context: ValidationContext) { +export function checkReference( + fieldToCheck: Field, + baselineField: Field, + context: ValidationContext, +) { if (fieldToCheck.isReference && !baselineField.isReference) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'REFERENCE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be a reference${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.referenceAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { + location: fieldToCheck.referenceAstNode, + }, ), ); return; @@ -34,7 +42,8 @@ export function checkReference(fieldToCheck: Field, baselineField: Field, contex // missing reference if (!fieldToCheck.isReference) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'REFERENCE', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with ${expectedReferenceDeclaration}${getRequiredBySuffix( @@ -53,9 +62,13 @@ export function checkReference(fieldToCheck: Field, baselineField: Field, contex baselineField.referenceKeyField === baselineField ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'REFERENCE', `Reference should not declare a keyField${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.referenceAstNode, + fieldToCheck.astNode, + { + location: fieldToCheck.referenceAstNode, + }, ), ); return; @@ -71,11 +84,15 @@ export function checkReference(fieldToCheck: Field, baselineField: Field, contex baselineField.getReferenceKeyFieldOrThrow().name ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'REFERENCE', `Reference should declare ${expectedKeyFieldDeclaration}${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.referenceAstNode, + fieldToCheck.astNode, + { + location: fieldToCheck.referenceAstNode, + }, ), ); } diff --git a/src/model/compatibility-check/check-relation.ts b/src/model/compatibility-check/check-relation.ts index 43173136..c428a317 100644 --- a/src/model/compatibility-check/check-relation.ts +++ b/src/model/compatibility-check/check-relation.ts @@ -14,11 +14,13 @@ export function checkRelation( // superfluous relation if (fieldToCheck.isRelation && !baselineField.isRelation) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'RELATION', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be a relation${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.relationAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.relationAstNode }, ), ); return; @@ -46,7 +48,8 @@ export function checkRelation( ? `@relation(${expectedInverseOfDeclaration}${expectedOnDeleteDeclaration})` : `@relation`; context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'RELATION', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with ${expectedRelationDeclaration}${getRequiredBySuffix( @@ -61,13 +64,15 @@ export function checkRelation( // superfluous inverseOf if (fieldToCheck.inverseOf && !baselineField.inverseOf) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'RELATION', `Relation "${ baselineField.name }" should be a forward relation, not an inverse relation${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.inverseOfAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.inverseOfAstNode }, ), ); } @@ -78,15 +83,15 @@ export function checkRelation( (!fieldToCheck.inverseOf || fieldToCheck.inverseOf.name !== baselineField.inverseOf.name) ) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'RELATION', `Relation "${ baselineField.name }" should be an inverse relation with ${expectedInverseOfDeclaration}${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.inverseOfAstNode ?? - fieldToCheck.relationAstNode ?? - fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.inverseOfAstNode ?? fieldToCheck.relationAstNode }, ), ); } @@ -98,13 +103,16 @@ export function checkRelation( ? `specify ${expectedOnDeleteDeclaration}` : `omit the "onDelete" argument`; context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'RELATION', `Relation "${baselineField.name}" should ${hint}${getRequiredBySuffix( baselineField, )}.`, - fieldToCheck.relationDeleteActionAstNode ?? - fieldToCheck.relationAstNode ?? - fieldToCheck.astNode, + fieldToCheck.astNode, + { + location: + fieldToCheck.relationDeleteActionAstNode ?? fieldToCheck.relationAstNode, + }, ), ); } diff --git a/src/model/compatibility-check/check-root-and-parent-directives.ts b/src/model/compatibility-check/check-root-and-parent-directives.ts index 4c75f571..339fc31d 100644 --- a/src/model/compatibility-check/check-root-and-parent-directives.ts +++ b/src/model/compatibility-check/check-root-and-parent-directives.ts @@ -12,18 +12,21 @@ export function checkRootAndParentDirectives( ) { if (fieldToCheck.isRootField && !baselineField.isRootField) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'ROOT_FIELD', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be decorated with @root${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.rootDirectiveAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.rootDirectiveAstNode }, ), ); } if (!fieldToCheck.isRootField && baselineField.isRootField) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'ROOT_FIELD', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with @root${getRequiredBySuffix(baselineField)}.`, @@ -34,18 +37,21 @@ export function checkRootAndParentDirectives( if (fieldToCheck.isParentField && !baselineField.isParentField) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'PARENT_FIELD', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should not be decorated with @parent${getRequiredBySuffix(baselineField)}.`, - fieldToCheck.rootDirectiveAstNode ?? fieldToCheck.astNode, + fieldToCheck.astNode, + { location: fieldToCheck.parentDirectiveAstNode }, ), ); } if (!fieldToCheck.isParentField && baselineField.isParentField) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'PARENT_FIELD', `Field "${baselineField.declaringType.name}.${ baselineField.name }" should be decorated with @parent${getRequiredBySuffix(baselineField)}.`, diff --git a/src/model/compatibility-check/check-root-entity-type.ts b/src/model/compatibility-check/check-root-entity-type.ts index 84955afc..bafed6d1 100644 --- a/src/model/compatibility-check/check-root-entity-type.ts +++ b/src/model/compatibility-check/check-root-entity-type.ts @@ -10,11 +10,13 @@ export function checkRootEntityType( ) { if (baselineType.isBusinessObject && !typeToCheck.isBusinessObject) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'BUSINESS_OBJECT', `Type "${ baselineType.name }" needs to be decorated with @businessObject${getRequiredBySuffix(baselineType)}.`, - typeToCheck.nameASTNode, + typeToCheck.astNode, + { location: typeToCheck.nameASTNode }, ), ); } diff --git a/src/model/compatibility-check/check-type.ts b/src/model/compatibility-check/check-type.ts index b68523f7..25f976a4 100644 --- a/src/model/compatibility-check/check-type.ts +++ b/src/model/compatibility-check/check-type.ts @@ -8,11 +8,14 @@ import { describeTypeKind } from './utils'; export function checkType(typeToCheck: Type, baselineType: Type, context: ValidationContext) { if (typeToCheck.kind !== baselineType.kind) { context.addMessage( - ValidationMessage.compatibilityIssue( + ValidationMessage.suppressableCompatibilityIssue( + 'TYPE_KIND', `Type "${baselineType.name}" needs to be ${describeTypeKind( baselineType.kind, )}${getRequiredBySuffix(baselineType)}.`, - typeToCheck.nameASTNode, + + typeToCheck.astNode, + { location: typeToCheck.nameASTNode }, ), ); return; diff --git a/src/model/create-model.ts b/src/model/create-model.ts index a5e5a180..d9c8cad2 100644 --- a/src/model/create-model.ts +++ b/src/model/create-model.ts @@ -124,6 +124,7 @@ import { parseI18nConfigs } from './parse-i18n'; import { parseModuleConfigs } from './parse-modules'; import { parseTTLConfigs } from './parse-ttl'; import { ValidationContext, ValidationMessage } from './validation'; +import { WarningCode } from '../schema/message-codes'; export function createModel(parsedProject: ParsedProject, options: ModelOptions = {}): Model { const validationContext = new ValidationContext(); @@ -307,7 +308,8 @@ function processKeyField( keyFieldASTNode = underscoreKeyField.astNode; keyFieldName = ID_FIELD; context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED', `The field "_key" is deprecated and should be replaced with "id" (of type "ID").`, underscoreKeyField.astNode, ), @@ -345,6 +347,7 @@ function getDefaultValue(fieldNode: FieldDefinitionNode, context: ValidationCont function getFlexSearchOrder( rootEntityDirective: DirectiveNode, + objectNode: ObjectTypeDefinitionNode, context: ValidationContext, ): ReadonlyArray { const argumentNode: ArgumentNode | undefined = getNodeByName( @@ -357,10 +360,12 @@ function getFlexSearchOrder( } if (argumentNode.value.kind === Kind.LIST) { - return argumentNode.value.values.map((v) => createFlexSearchPrimarySortClause(v, context)); + return argumentNode.value.values.map((v) => + createFlexSearchPrimarySortClause(v, objectNode, context), + ); } else { // graphql syntax allows list values to be defined without [] which results in an OBJECT - return [createFlexSearchPrimarySortClause(argumentNode.value, context)]; + return [createFlexSearchPrimarySortClause(argumentNode.value, objectNode, context)]; } } @@ -412,6 +417,7 @@ function getFlexSearchPerformanceParams( function createFlexSearchPrimarySortClause( valueNode: ValueNode, + objectTypeNode: ObjectTypeDefinitionNode, context: ValidationContext, ): FlexSearchPrimarySortClauseConfig { if (valueNode.kind !== Kind.OBJECT) { @@ -437,9 +443,13 @@ function createFlexSearchPrimarySortClause( // a missing direction was just silently be ignored and assumed to be ASC in the past // for a migration period, treat it as a warning context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED', `Field "FlexSearchOrderArgument.direction" of required type "OrderDirection!" was not provided. "ASC" will be assumed. This will be an error in a future release.`, - valueNode, + objectTypeNode, + { + location: valueNode, + }, ), ); direction = OrderDirection.ASCENDING; @@ -485,7 +495,7 @@ function createFlexSearchDefinitionInputs( return { isIndexed, directiveASTNode: directive, - primarySort: directive ? getFlexSearchOrder(directive, context) : [], + primarySort: directive ? getFlexSearchOrder(directive, objectNode, context) : [], performanceParams: directive ? getFlexSearchPerformanceParams(directive) : undefined, }; } diff --git a/src/model/implementation/enum-type.ts b/src/model/implementation/enum-type.ts index 43e0087b..22c8507a 100644 --- a/src/model/implementation/enum-type.ts +++ b/src/model/implementation/enum-type.ts @@ -6,6 +6,7 @@ import { EnumValueLocalization } from './i18n'; import { Model } from './model'; import { TypeBase } from './type-base'; import memorize from 'memorize-decorator'; +import { WarningCode } from '../../schema/message-codes'; export class EnumType extends TypeBase { constructor(input: EnumTypeConfig, model: Model) { @@ -63,7 +64,11 @@ export class EnumValue implements ModelComponent { } if (this.value.toUpperCase() !== this.value) { context.addMessage( - ValidationMessage.warn(`Enum values should be UPPER_CASE.`, this.astNode), + ValidationMessage.suppressableWarning( + 'NAMING', + `Enum values should be UPPER_CASE.`, + this.astNode, + ), ); } } diff --git a/src/model/implementation/field.ts b/src/model/implementation/field.ts index e0724835..d13b14ff 100644 --- a/src/model/implementation/field.ts +++ b/src/model/implementation/field.ts @@ -55,6 +55,7 @@ import { Relation, RelationSide } from './relation'; import { RolesSpecifier } from './roles-specifier'; import { InvalidType, ObjectType, Type } from './type'; import { ValueObjectType } from './value-object-type'; +import { WarningCode } from '../../schema/message-codes'; export interface SystemFieldConfig extends FieldConfig { readonly isSystemField?: boolean; @@ -134,7 +135,7 @@ export class Field implements ModelComponent { this.calcMutationOperators = new Set(input.calcMutationOperators || []); this.roles = input.permissions && input.permissions.roles - ? new RolesSpecifier(input.permissions.roles) + ? new RolesSpecifier(input.permissions.roles, this) : undefined; this.isSystemField = input.isSystemField || false; this.isAccessField = input.isAccessField ?? false; @@ -418,14 +419,19 @@ export class Field implements ModelComponent { if (this.name.includes('_')) { context.addMessage( - ValidationMessage.warn(`Field names should not include underscores.`, this.astNode), + ValidationMessage.suppressableWarning( + 'NAMING', + `Field names should not include underscores.`, + this.astNode, + ), ); return; } if (!this.name.match(/^[a-z]/)) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'NAMING', `Field names should start with a lowercase character.`, this.astNode, ), @@ -581,7 +587,8 @@ export class Field implements ModelComponent { ); if (matchingRelation) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'MIRRORED_RELATIONS', `This field and "${matchingRelation.declaringType.name}.${matchingRelation.name}" define separate relations. Consider using the "inverseOf" argument to add a backlink to an existing relation.`, this.astNode, ), @@ -711,7 +718,8 @@ export class Field implements ModelComponent { } else if (!this.input.referenceKeyField) { // can only format this nicely if we have the key field context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED_REFERENCE', `Usage of @reference without the keyField argument is deprecated. Add a field of type "${this.type.keyField.type.name}" and specify it in @reference(keyField: "...")`, this.astNode, ), @@ -815,9 +823,11 @@ export class Field implements ModelComponent { } // even for boolean lists, we show a warning because boolean lists are sparingly used and e.g. using EVERY might be misleading there + // TODO don't we need to check if teh aggregation operator already is *_TRUE, or would we not take this branch then? if (lastSegment && lastSegment.field.type.name === GraphQLBoolean.name) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'BOOLEAN_AGGREGATION', `Aggregation operator "${ this.aggregationOperator }" only checks the number of items. "${ @@ -825,7 +835,8 @@ export class Field implements ModelComponent { }" is of type "Boolean", so you may want to use the operator "${ this.aggregationOperator + '_TRUE' }" instead which specifically checks for boolean "true".`, - this.input.collect.aggregationOperatorASTNode, + this.input.astNode, + { location: this.input.collect.aggregationOperatorASTNode }, ), ); } @@ -1301,9 +1312,11 @@ export class Field implements ModelComponent { } context.addMessage( - ValidationMessage.info( + ValidationMessage.suppressableInfo( + 'NO_TYPE_CHECKS', `Take care, there are no type checks for default values yet.`, - this.input.defaultValueASTNode || this.astNode, + this.astNode, + { location: this.input.defaultValueASTNode || this.astNode }, ), ); } @@ -1359,7 +1372,8 @@ export class Field implements ModelComponent { if (this.declaringType.isValueObjectType) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED', `Calc mutations do not work within value objects because value objects cannot be updated. This will be an error in a future release.`, this.astNode, ), @@ -1531,9 +1545,11 @@ export class Field implements ModelComponent { } } context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'UNSUPPORTED_PARENT_FIELD', `Parent fields currently can't be selected within collect fields, so this field will probably be useless.${rootNote}`, - this.input.parentDirectiveNode, + this.astNode, + { location: this.input.parentDirectiveNode }, ), ); } @@ -1734,9 +1750,11 @@ export class Field implements ModelComponent { // this used to be accepted silently in the past // report a warning for the transition period, and change this to an error later context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED', `@flexSearch is not supported on type "${this.type.name}". Remove this directive. This will be an error in a future release.`, - this.input.isFlexSearchIndexedASTNode, + this.input.astNode, + { location: this.input.isFlexSearchIndexedASTNode }, ), ); } @@ -1785,10 +1803,16 @@ export class Field implements ModelComponent { ) ) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'INEFFECTIVE_INCLUDE_IN_SEARCH', `"includeInSearch: true" does not have an effect because none of the fields in type "${this.type.name}" have "includeInSearch: true".`, - this.input.isFlexSearchIndexedASTNode ?? - this.input.isFlexSearchFulltextIndexedASTNode, + + this.input.astNode, + { + location: + this.input.isFlexSearchIndexedASTNode ?? + this.input.isFlexSearchFulltextIndexedASTNode, + }, ), ); } diff --git a/src/model/implementation/flex-search.ts b/src/model/implementation/flex-search.ts index 8c655a38..b3806ff5 100644 --- a/src/model/implementation/flex-search.ts +++ b/src/model/implementation/flex-search.ts @@ -4,6 +4,7 @@ import { ModelComponent, ValidationContext } from '../validation/validation-cont import { FlexSearchPrimarySortClauseConfig } from '../config'; import { RootEntityType } from './root-entity-type'; import { Severity, ValidationMessage } from '../validation'; +import { WarningCode } from '../../schema/message-codes'; export const IDENTITY_ANALYZER = 'identity'; export const NORM_CI_ANALYZER = 'norm_ci'; @@ -14,7 +15,7 @@ export class FlexSearchPrimarySortClause implements ModelComponent { constructor( private readonly config: FlexSearchPrimarySortClauseConfig, - baseType: RootEntityType, + private readonly baseType: RootEntityType, ) { this.field = new FieldPath({ path: config.field, @@ -34,12 +35,13 @@ export class FlexSearchPrimarySortClause implements ModelComponent { // we did not report any errors previously. In a transition period, we make it clear // that these warnings will be errors in the future. context.addMessage( - new ValidationMessage( - Severity.WARNING, + ValidationMessage.suppressableWarning( + 'DEPRECATED', message.message + (message.message.endsWith('.') ? '' : '.') + ' This will be an error in a future release.', - message.location, + this.baseType.astNode, + { location: message.location }, ), ); } else { @@ -56,9 +58,11 @@ export class FlexSearchPrimarySortClause implements ModelComponent { if (!lastField.type.isScalarType && !lastField.type.isEnumType) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'DEPRECATED', `Field "${lastField.declaringType.name}.${lastField.name}" is an object field, but only scalar and enum fields are supported in flexSearchOrder. Choose a subfield or a different field. This will be an error in a future release.`, - this.config.fieldASTNode, + this.baseType.astNode, + { location: this.config.fieldASTNode }, ), ); } diff --git a/src/model/implementation/i18n.ts b/src/model/implementation/i18n.ts index a9f6246d..1f62523e 100644 --- a/src/model/implementation/i18n.ts +++ b/src/model/implementation/i18n.ts @@ -365,7 +365,7 @@ function checkForTypeConstraints( if (!modelType) { validationContext.addMessage( - ValidationMessage.warn( + ValidationMessage.nonSuppressableWarning( 'There is no type "' + typeKey + '" in the model specification. This might be a spelling error.', @@ -381,7 +381,7 @@ function checkForTypeConstraints( for (const field in type.fields) { if (!objectType.fields.find((f) => f.name === field)) { validationContext.addMessage( - ValidationMessage.warn( + ValidationMessage.nonSuppressableWarning( 'The type "' + typeKey + '" has no field "' + @@ -418,7 +418,7 @@ function checkForTypeConstraints( for (const value in type.values) { if (!enumType.values.find((v) => v.value === value)) { validationContext.addMessage( - ValidationMessage.warn( + ValidationMessage.nonSuppressableWarning( 'The enum type "' + typeKey + '" has no value "' + diff --git a/src/model/implementation/namespace.ts b/src/model/implementation/namespace.ts index caf9cf24..fcc89766 100644 --- a/src/model/implementation/namespace.ts +++ b/src/model/implementation/namespace.ts @@ -238,7 +238,7 @@ export class Namespace implements ModelComponent { const shadowed = this.parent.getPermissionProfile(profile.name); if (shadowed) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.nonSuppressableWarning( `Permission profile: "${ profile.name }" shadows the profile in namespace "${shadowed.namespacePath.join( diff --git a/src/model/implementation/object-type-base.ts b/src/model/implementation/object-type-base.ts index 0945fb5e..5bdbe4e0 100644 --- a/src/model/implementation/object-type-base.ts +++ b/src/model/implementation/object-type-base.ts @@ -93,8 +93,7 @@ export abstract class ObjectTypeBase extends TypeBase { const systemField = this.getSystemFieldOrThrow(systemFieldOverride.name); if (systemField.type.name !== systemFieldOverride.typeName) { context.addMessage( - new ValidationMessage( - Severity.ERROR, + ValidationMessage.error( `System field "${systemField.name}" must be of type "${systemField.type.name}"`, systemField.astNode, ), @@ -103,8 +102,8 @@ export abstract class ObjectTypeBase extends TypeBase { if (!systemFieldOverride.astNode?.directives?.length) { context.addMessage( - new ValidationMessage( - Severity.WARNING, + ValidationMessage.suppressableWarning( + 'REDUNDANT_FIELD', `Manually declaring system field "${systemField.name}" is redundant. Either add a suitable directive or consider removing the field`, systemField.astNode, ), @@ -119,8 +118,7 @@ export abstract class ObjectTypeBase extends TypeBase { ) ?? []; for (const forbiddenDirective of forbiddenSystemFieldDirectives) { context.addMessage( - new ValidationMessage( - Severity.ERROR, + ValidationMessage.error( `Directive "@${forbiddenDirective.name.value}" is not allowed on system field "${systemFieldOverride.name}" and will be discarded`, forbiddenDirective, ), diff --git a/src/model/implementation/roles-specifier.ts b/src/model/implementation/roles-specifier.ts index 8ea8bb5a..6f15e20b 100644 --- a/src/model/implementation/roles-specifier.ts +++ b/src/model/implementation/roles-specifier.ts @@ -1,12 +1,18 @@ import { ModelComponent, ValidationContext } from '../validation/validation-context'; import { RolesSpecifierConfig } from '../config'; import { ValidationMessage } from '../validation'; +import { WarningCode } from '../../schema/message-codes'; +import { Type } from './type'; +import { Field } from './field'; export class RolesSpecifier implements ModelComponent { readonly read: ReadonlyArray; readonly readWrite: ReadonlyArray; - constructor(private readonly input: RolesSpecifierConfig) { + constructor( + private readonly input: RolesSpecifierConfig, + private readonly declaringFieldOrType: Type | Field, + ) { this.read = input.read || []; this.readWrite = input.readWrite || []; } @@ -14,16 +20,23 @@ export class RolesSpecifier implements ModelComponent { validate(context: ValidationContext) { if (this.read.length === 0 && this.readWrite.length === 0) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'NO_ROLES', `No roles with read access are specified. Access is denied for everyone.`, - this.input.astNode, + this.declaringFieldOrType.astNode, + { location: this.input.astNode }, ), ); } if ([...this.read, ...this.readWrite].some((role) => role === '')) { context.addMessage( - ValidationMessage.warn(`Specified empty string as role.`, this.input.astNode), + ValidationMessage.suppressableWarning( + 'EMPTY_NAME', + `Specified empty string as role.`, + this.declaringFieldOrType.astNode, + { location: this.input.astNode }, + ), ); } } diff --git a/src/model/implementation/root-entity-type.ts b/src/model/implementation/root-entity-type.ts index 4dd852a4..f2814eef 100644 --- a/src/model/implementation/root-entity-type.ts +++ b/src/model/implementation/root-entity-type.ts @@ -21,7 +21,7 @@ import { RootEntityTypeConfig, TypeKind, } from '../config'; -import { ValidationContext, ValidationMessage } from '../validation'; +import { QuickFix, ValidationContext, ValidationMessage } from '../validation'; import { Field, SystemFieldConfig } from './field'; import { FieldPath } from './field-path'; import { FlexSearchPrimarySortClause } from './flex-search'; @@ -35,6 +35,7 @@ import { RolesSpecifier } from './roles-specifier'; import { ScalarType } from './scalar-type'; import { TimeToLiveType } from './time-to-live'; import { EffectiveModuleSpecification } from './modules/effective-module-specification'; +import { WarningCode } from '../../schema/message-codes'; export class RootEntityType extends ObjectTypeBase { private readonly permissions: PermissionsConfig & {}; @@ -57,7 +58,7 @@ export class RootEntityType extends ObjectTypeBase { this.permissions = input.permissions || {}; this.roles = input.permissions && input.permissions.roles - ? new RolesSpecifier(input.permissions.roles) + ? new RolesSpecifier(input.permissions.roles, this) : undefined; this.isBusinessObject = input.isBusinessObject || false; if (input.flexSearchIndexConfig && input.flexSearchIndexConfig.isIndexed) { @@ -397,18 +398,20 @@ export class RootEntityType extends ObjectTypeBase { // (definition is useful because it shows the exact path within the field that is wrong) for (const message of nestedContext.validationMessages) { context.addMessage( - new ValidationMessage( - message.severity, - `Permission profile "${this.permissionProfile.name}" defines restrictions that cannot be applied to type "${this.name}": ${message.message}`, - this.input.permissions?.permissionProfileNameAstNode, - ), + new ValidationMessage({ + severity: message.severity, + message: `Permission profile "${this.permissionProfile.name}" defines restrictions that cannot be applied to type "${this.name}": ${message.message}`, + location: this.input.permissions?.permissionProfileNameAstNode, + quickFixes: message.quickFixes, + }), ); context.addMessage( - new ValidationMessage( - message.severity, - `Cannot be applied to type "${this.name}": ${message.message}`, - message.location, - ), + new ValidationMessage({ + severity: message.severity, + message: `Cannot be applied to type "${this.name}": ${message.message}`, + location: message.location, + quickFixes: message.quickFixes, + }), ); } } @@ -433,9 +436,11 @@ export class RootEntityType extends ObjectTypeBase { ) ) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'INEFFECTIVE_FLEX_SEARCH', `The type contains fields that are annotated with ${FLEX_SEARCH_INDEXED_DIRECTIVE} or ${FLEX_SEARCH_FULLTEXT_INDEXED_DIRECTIVE}, but the type itself is not marked with flexSearch = true.`, - this.input.astNode!.name, + this.input.astNode, + { location: this.input.astNode?.name }, ), ); } diff --git a/src/model/implementation/time-to-live.ts b/src/model/implementation/time-to-live.ts index 1117276c..3bc94c57 100644 --- a/src/model/implementation/time-to-live.ts +++ b/src/model/implementation/time-to-live.ts @@ -11,6 +11,7 @@ import { RootEntityType } from './root-entity-type'; import { ScalarType } from './scalar-type'; import { Type } from './type'; import { FieldPath } from './field-path'; +import { WarningCode } from '../../schema/message-codes'; export class TimeToLiveType implements ModelComponent { readonly cascadeFields: ReadonlyArray = []; @@ -161,7 +162,7 @@ export class TimeToLiveType implements ModelComponent { if (cascadeField.lastField.relationDeleteAction === RelationDeleteAction.CASCADE) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.nonSuppressableWarning( `Field ${lastFieldDesc} is already annotated with @relation(onDelete=CASCADE) and listing it here does not have an effect.`, cascadeField.location, ), diff --git a/src/model/implementation/type-base.ts b/src/model/implementation/type-base.ts index 820ec236..a8997052 100644 --- a/src/model/implementation/type-base.ts +++ b/src/model/implementation/type-base.ts @@ -11,6 +11,7 @@ import { EffectiveModuleSpecification } from './modules/effective-module-specifi import { MODULES_DIRECTIVE } from '../../schema/constants'; import { TypeModuleSpecification } from './modules/type-module-specification'; import { Type } from './type'; +import { WarningCode } from '../../schema/message-codes'; export abstract class TypeBase implements ModelComponent { readonly name: string; @@ -78,9 +79,11 @@ export abstract class TypeBase implements ModelComponent { if (this.name.includes('_')) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'NAMING', `Type names should not include underscores.`, - this.nameASTNode, + this.astNode, + { location: this.nameASTNode }, ), ); return; @@ -88,9 +91,11 @@ export abstract class TypeBase implements ModelComponent { if (!this.name.match(/^[A-Z]/)) { context.addMessage( - ValidationMessage.warn( + ValidationMessage.suppressableWarning( + 'NAMING', `Type names should start with an uppercase character.`, - this.nameASTNode, + this.astNode, + { location: this.nameASTNode }, ), ); } diff --git a/src/model/validation/location.ts b/src/model/validation/location.ts index 71333b31..73e3cfd8 100644 --- a/src/model/validation/location.ts +++ b/src/model/validation/location.ts @@ -105,7 +105,7 @@ export class MessageLocation { } } -function isASTNode(obj: LocationLike): obj is ASTNode { +export function isASTNode(obj: LocationLike): obj is ASTNode { return 'kind' in obj; } diff --git a/src/model/validation/message.ts b/src/model/validation/message.ts index f6d54f18..e4b9a1f2 100644 --- a/src/model/validation/message.ts +++ b/src/model/validation/message.ts @@ -1,3 +1,16 @@ +import { EnumValueDefinitionNode, FieldDefinitionNode, Kind, TypeDefinitionNode } from 'graphql'; +import { + SUPPRESS_COMPATIBILITY_ISSUES_ARG, + SUPPRESS_DIRECTIVE, + SUPPRESS_INFOS_ARG, + SUPPRESS_WARNINGS_ARG, +} from '../../schema/constants'; +import { + CompatibilityIssueCode, + InfoCode, + MessageCode, + WarningCode, +} from '../../schema/message-codes'; import { LocationLike, MessageLocation } from './location'; import { QuickFix } from './quick-fix'; @@ -12,18 +25,60 @@ export interface ValidationMessageOptions { readonly quickFixes?: ReadonlyArray; } +export interface SuppressableValidationMessageOptions extends ValidationMessageOptions { + /** + * The location where to report the message, in case it differs from the astNode + */ + readonly location?: LocationLike; +} + +export interface ValidationMessageParams { + readonly severity: Severity; + readonly message: string; + readonly quickFixes?: ReadonlyArray; + readonly location?: LocationLike; + readonly code?: MessageCode; + readonly isSuppressed?: boolean; +} + +export type AstNodeWithDirectives = + | FieldDefinitionNode + | TypeDefinitionNode + | EnumValueDefinitionNode; + export class ValidationMessage { + readonly severity: Severity; + readonly message: string; readonly location: MessageLocation | undefined; readonly quickFixes: ReadonlyArray; + readonly isSuppressed: boolean; - constructor( - public readonly severity: Severity, - public readonly message: string, - location: LocationLike | undefined, - options: ValidationMessageOptions = {}, + constructor(params: ValidationMessageParams) { + this.severity = params.severity; + this.message = params.message; + this.location = params.location ? MessageLocation.from(params.location) : undefined; + this.quickFixes = params.quickFixes ?? []; + this.isSuppressed = params.isSuppressed ?? false; + } + + public static suppressableMessage( + severity: Severity.WARNING | Severity.INFO | Severity.COMPATIBILITY_ISSUE, + code: MessageCode, + message: string, + astNode: AstNodeWithDirectives | undefined, + options?: SuppressableValidationMessageOptions, ) { - this.location = location ? MessageLocation.from(location) : undefined; - this.quickFixes = options.quickFixes ?? []; + // not sure if this is the right time to do this + // also does not allow us to detect superfluous directives at the moment + const isSuppressed = calculateIsSuppressed(severity, astNode, code); + return new ValidationMessage({ + severity, + code, + message, + location: options?.location ?? astNode, + isSuppressed, + ...options, + }); } public static error( @@ -31,33 +86,92 @@ export class ValidationMessage { location: LocationLike | undefined, options?: ValidationMessageOptions, ) { - return new ValidationMessage(Severity.ERROR, message, location, options); + return new ValidationMessage({ severity: Severity.ERROR, message, location, ...options }); + } + + public static suppressableWarning( + code: WarningCode, + message: string, + astNode: AstNodeWithDirectives | undefined, + options?: SuppressableValidationMessageOptions, + ) { + return ValidationMessage.suppressableMessage( + Severity.WARNING, + code, + message, + astNode, + options, + ); } - public static compatibilityIssue( + public static nonSuppressableWarning( message: string, location: LocationLike | undefined, options?: ValidationMessageOptions, ) { - return new ValidationMessage(Severity.COMPATIBILITY_ISSUE, message, location, options); + return new ValidationMessage({ + severity: Severity.WARNING, + message, + location, + ...options, + }); + } + + public static suppressableInfo( + code: InfoCode, + message: string, + astNode: AstNodeWithDirectives | undefined, + options?: SuppressableValidationMessageOptions, + ) { + return ValidationMessage.suppressableMessage( + Severity.INFO, + code, + message, + astNode, + options, + ); } - public static warn( + public static nonSuppressableInfo( message: string, location: LocationLike | undefined, options?: ValidationMessageOptions, ) { - return new ValidationMessage(Severity.WARNING, message, location, options); + return new ValidationMessage({ + severity: Severity.INFO, + message, + location, + ...options, + }); + } + + public static suppressableCompatibilityIssue( + code: CompatibilityIssueCode, + message: string, + astNode: AstNodeWithDirectives | undefined, + options?: SuppressableValidationMessageOptions, + ) { + return ValidationMessage.suppressableMessage( + Severity.COMPATIBILITY_ISSUE, + code, + message, + astNode, + options, + ); } - public static info( + public static nonSuppressableCompatibilityIssue( message: string, location: LocationLike | undefined, options?: ValidationMessageOptions, ) { - return new ValidationMessage(Severity.INFO, message, location, options); + return new ValidationMessage({ + severity: Severity.COMPATIBILITY_ISSUE, + message, + location, + ...options, + }); } - public toString() { const at = this.location ? ` at ${this.location}` : ''; return `${severityToString(this.severity)}: ${this.message}${at}`; @@ -76,3 +190,42 @@ function severityToString(severity: Severity) { return 'Compatibility issue'; } } + +function calculateIsSuppressed( + severity: Severity, + location: AstNodeWithDirectives | undefined, + code: MessageCode, +) { + const suppressDirective = location?.directives?.find( + (d) => d.name.value === SUPPRESS_DIRECTIVE, + ); + if (!suppressDirective) { + return false; + } + let argName; + switch (severity) { + case Severity.COMPATIBILITY_ISSUE: + argName = SUPPRESS_COMPATIBILITY_ISSUES_ARG; + break; + case Severity.WARNING: + argName = SUPPRESS_WARNINGS_ARG; + break; + case Severity.INFO: + argName = SUPPRESS_INFOS_ARG; + break; + default: + throw new Error(`Non-suppressable severity: ${severity}`); + } + const codesArg = suppressDirective?.arguments?.find((a) => a.name.value === argName); + if (!codesArg) { + return false; + } + if (codesArg.value.kind === Kind.ENUM) { + // you can omit the [] in graphql if it's a single list entry + return codesArg.value.value === code; + } + if (codesArg.value.kind !== Kind.LIST) { + return false; + } + return codesArg.value.values.some((v) => v.kind === Kind.ENUM && v.value === code); +} diff --git a/src/model/validation/quick-fix.ts b/src/model/validation/quick-fix.ts index 5d403858..4346fb99 100644 --- a/src/model/validation/quick-fix.ts +++ b/src/model/validation/quick-fix.ts @@ -9,7 +9,7 @@ export interface QuickFixParams { /** * Whether this is likely a fix of the underlying problem * - * For example, a code fix to add a missing field would be preferred, while a code fix to add a @supress directive would not be preferred. + * For example, a code fix to add a missing field would be preferred, while a code fix to add a @suppress directive would not be preferred. * * Can be used by UIs to apply a quick fix via a hotkey, or to offer a "fix all issues" feature. */ @@ -36,7 +36,7 @@ export class QuickFix { /** * Whether this is likely a fix of the underlying problem * - * For example, a code fix to add a missing field would be preferred, while a code fix to add a @supress directive would not be preferred. + * For example, a code fix to add a missing field would be preferred, while a code fix to add a @suppress directive would not be preferred. * * Can be used by UIs to apply a quick fix via a hotkey, or to offer a "fix all issues" feature. * diff --git a/src/model/validation/result.ts b/src/model/validation/result.ts index 52faee97..9c9d47ac 100644 --- a/src/model/validation/result.ts +++ b/src/model/validation/result.ts @@ -1,7 +1,13 @@ import { Severity, ValidationMessage } from './message'; export class ValidationResult { - constructor(public readonly messages: ReadonlyArray) {} + readonly messages: ReadonlyArray; + readonly suppressedMessages: ReadonlyArray; + + constructor(public readonly allMessages: ReadonlyArray) { + this.suppressedMessages = allMessages.filter((m) => m.isSuppressed); + this.messages = allMessages.filter((m) => !m.isSuppressed); + } public hasMessages() { return this.messages.length > 0; diff --git a/src/schema/constants.ts b/src/schema/constants.ts index 14ab325a..fe57a3ad 100644 --- a/src/schema/constants.ts +++ b/src/schema/constants.ts @@ -44,6 +44,11 @@ export const MODULES_IN_ARG = 'in'; export const MODULES_ALL_ARG = 'all'; export const MODULES_INCLUDE_ALL_FIELDS_ARG = 'includeAllFields'; +export const SUPPRESS_DIRECTIVE = 'suppress'; +export const SUPPRESS_COMPATIBILITY_ISSUES_ARG = 'compatibilityIssues'; +export const SUPPRESS_WARNINGS_ARG = 'warnings'; +export const SUPPRESS_INFOS_ARG = 'infos'; + export const QUERY_TYPE = 'Query'; export const MUTATION_TYPE = 'Mutation'; export const QUERY_META_TYPE = '_QueryMeta'; diff --git a/src/schema/graphql-base.ts b/src/schema/graphql-base.ts index 83fd9cd4..ef467769 100644 --- a/src/schema/graphql-base.ts +++ b/src/schema/graphql-base.ts @@ -1,7 +1,19 @@ -import { DocumentNode } from 'graphql'; +import { + DocumentNode, + EnumTypeDefinitionNode, + EnumValueDefinitionNode, + Kind, + TypeKind, +} from 'graphql'; import gql from 'graphql-tag'; - -export const DIRECTIVES: DocumentNode = gql` +import { + COMPATIBILITY_ISSUE_CODES, + INFO_CODES, + MessageCodes, + WARNING_CODES, +} from './message-codes'; + +const directivesBase: DocumentNode = gql` "Declares a type for root-level objects with ids that are stored directly in the data base" directive @rootEntity( indices: [IndexDefinition!] @@ -267,8 +279,72 @@ export const DIRECTIVES: DocumentNode = gql` "Specifies that all fields in this type should be included in all modules declared on this type." includeAllFields: Boolean ) on OBJECT | ENUM | FIELD_DEFINITION + + "Disables one or multiple non-error messages on a type, field or other declaration" + directive @suppress( + warnings: [WarningCode!] + infos: [InfoCode!] + compatibilityIssues: [CompatibilityIssueCode!] + ) on OBJECT | ENUM | FIELD_DEFINITION + + "Codes that can be used with @suppress(warnings: [...])" + enum WarningCode { + DUMMY + } + + "Codes that can be used with @suppress(infos: [...])" + enum InfoCode { + DUMMY + } + + "Codes that can be used with @suppress(compatibilityIssues: [...])" + enum CompatibilityIssueCode { + DUMMY + } `; +export const DIRECTIVES = generateDirectivesAst(); + +function generateDirectivesAst(): DocumentNode { + return { + ...directivesBase, + definitions: directivesBase.definitions.map((def) => { + if (def.kind !== Kind.ENUM_TYPE_DEFINITION) { + return def; + } + switch (def.name.value) { + case 'WarningCode': + return buildEnumValues(def, WARNING_CODES); + case 'InfoCode': + return buildEnumValues(def, INFO_CODES); + case 'CompatibilityIssueCode': + return buildEnumValues(def, COMPATIBILITY_ISSUE_CODES); + default: + return def; + } + }), + }; +} + +function buildEnumValues(baseDefinitions: EnumTypeDefinitionNode, codes: MessageCodes) { + return { + ...baseDefinitions, + values: Object.entries(codes).map( + ([name, description]): EnumValueDefinitionNode => ({ + kind: Kind.ENUM_VALUE_DEFINITION, + name: { + kind: Kind.NAME, + value: name, + }, + description: { + kind: Kind.STRING, + value: description as string, + }, + }), + ), + }; +} + export const CORE_SCALARS: DocumentNode = gql` """ The \`DateTime\` scalar type represents a point in time in UTC, in a format specified by ISO 8601, such as \`2007-12-03T10:15:30Z\` or \`2007-12-03T10:15:30.123Z\`. diff --git a/src/schema/message-codes.ts b/src/schema/message-codes.ts new file mode 100644 index 00000000..e8a387fa --- /dev/null +++ b/src/schema/message-codes.ts @@ -0,0 +1,62 @@ +/** + * A map of codes to their descriptions + */ +export type MessageCodes = { readonly [code: string]: string }; + +export const WARNING_CODES = { + NAMING: 'A naming convention has been violated', + + DEPRECATED: 'A deprecated feature is used', + + UNUSED: 'A type is declared but not used', + + MIRRORED_RELATIONS: + 'Two relations are the exact mirror of each other. A "inverseOf" on one of them might be missing.', + + // Separate from DEPRECATED because this is not planned to be removed yet + DEPRECATED_REFERENCE: '@reference is used without keyField', + + BOOLEAN_AGGREGATION: 'A @collect field that aggregates booleans in a possibly unintended way', + + UNSUPPORTED_PARENT_FIELD: 'A @parent field that is currently not usable', + + INEFFECTIVE_FLEX_SEARCH: + '@flexSearch or @flexSearchFulltext is specified but does not have an effect', + + INEFFECTIVE_INCLUDE_IN_SEARCH: + '@flexSearch(includeInSearch: true) is specified but does not have an effect', + + NO_ROLES: '@roles is specified but no roles are allowed, so access is denied for everyone', + + EMPTY_NAME: 'The empty string is used as a name', + + REDUNDANT_FIELD: 'A field declaration that is not needed', +} as const satisfies MessageCodes; + +export type WarningCode = keyof typeof WARNING_CODES; + +export const INFO_CODES = { + NO_TYPE_CHECKS: '@defaultValue currently does not have type checks for the value', +} as const satisfies MessageCodes; + +export type InfoCode = keyof typeof INFO_CODES; + +export const COMPATIBILITY_ISSUE_CODES = { + CALC_MUTATIONS: 'Missing @calcMutations operators', + COLLECT: 'Missing, superfluous or diverging @collect', + DEFAULT_VALUE: 'Missing, superfluous or diverging @defaultValue', + MISSING_ENUM_VALUE: 'An enum declaration is missing a value', + FIELD_TYPE: 'A field has the wrong type', + MISSING_FIELD: 'An object type declaration is missing a field', + KEY_FIELD: 'Missing or superfluous @key', + REFERENCE: 'Missing, superfluous or diverging @reference', + RELATION: 'Missing, superfluous or diverging @relation', + ROOT_FIELD: 'Missing or superfluous @root', + PARENT_FIELD: 'Missing or superfluous @parent', + BUSINESS_OBJECT: 'Missing or superfluous @businessObject', + TYPE_KIND: 'A type declaration is of the wrong kind (e.g. root entity, value object or enum)', +} as const satisfies MessageCodes; + +export type CompatibilityIssueCode = keyof typeof COMPATIBILITY_ISSUE_CODES; + +export type MessageCode = WarningCode | InfoCode | CompatibilityIssueCode; diff --git a/src/schema/preparation/ast-validation-modules/no-unused-non-root-object-types-validator.ts b/src/schema/preparation/ast-validation-modules/no-unused-non-root-object-types-validator.ts index 195b429f..be47d0f5 100644 --- a/src/schema/preparation/ast-validation-modules/no-unused-non-root-object-types-validator.ts +++ b/src/schema/preparation/ast-validation-modules/no-unused-non-root-object-types-validator.ts @@ -7,6 +7,7 @@ import { getTypeNameIgnoringNonNullAndList, } from '../../schema-utils'; import { ASTValidator } from '../ast-validator'; +import { WarningCode } from '../../message-codes'; export class NoUnusedNonRootObjectTypesValidator implements ASTValidator { validate(ast: DocumentNode): ReadonlyArray { @@ -31,7 +32,12 @@ export class NoUnusedNonRootObjectTypesValidator implements ASTValidator { ); // remaining object types in set are unused, create warnings for them return Array.from(objectTypeNames).map((unusedType) => - ValidationMessage.warn(`Type "${unusedType.name.value}" is not used.`, unusedType.name), + ValidationMessage.suppressableWarning( + 'UNUSED', + `Type "${unusedType.name.value}" is not used.`, + unusedType, + { location: unusedType.name }, + ), ); } } diff --git a/src/schema/preparation/source-validation-modules/sidecar-schema.ts b/src/schema/preparation/source-validation-modules/sidecar-schema.ts index 780cf4e1..207f54c8 100644 --- a/src/schema/preparation/source-validation-modules/sidecar-schema.ts +++ b/src/schema/preparation/source-validation-modules/sidecar-schema.ts @@ -26,9 +26,9 @@ export class SidecarSchemaValidator implements ParsedSourceValidator { if (isWarning) { if (path in source.pathLocationMap) { const loc = source.pathLocationMap[path]; - return ValidationMessage.warn(err.message!, loc); + return ValidationMessage.nonSuppressableWarning(err.message!, loc); } else { - return ValidationMessage.warn( + return ValidationMessage.nonSuppressableWarning( `${err.message} (at ${err.instancePath})`, undefined, ); diff --git a/src/schema/schema-builder.ts b/src/schema/schema-builder.ts index 7629cd19..4b911512 100644 --- a/src/schema/schema-builder.ts +++ b/src/schema/schema-builder.ts @@ -200,10 +200,10 @@ function parseYAMLSource( const severity = error.isWarning ? Severity.WARNING : Severity.ERROR; const endPos = getLineEndPosition(error.mark.line + 1, projectSource); validationContext.addMessage( - new ValidationMessage( + new ValidationMessage({ severity, - error.reason, - new MessageLocation( + message: error.reason, + location: new MessageLocation( projectSource, new SourcePosition( error.mark.position, @@ -212,7 +212,7 @@ function parseYAMLSource( ), endPos, ), - ), + }), ); });