From 4a3cf562176f981e278aff89cb58a05cf51eb0c6 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 22 Oct 2023 19:10:26 +0200 Subject: [PATCH 1/2] feat: error if pipeline file is in a `safeds.xy` package --- src/language/validation/other/modules.ts | 39 ++++++++++++------- src/language/validation/safe-ds-validator.ts | 7 +++- .../pipe elsewhere.sdspipe | 3 ++ .../pipe in safeds.sdspipe | 3 ++ .../pipe in subpackage of safeds.sdspipe | 3 ++ .../stub elsewhere.sdsstub | 3 ++ .../stub in safeds.sdsstub | 3 ++ .../stub in subpackage of safeds.sdsstub | 3 ++ .../test elsewhere.sdstest | 3 ++ .../test in safeds.sdstest | 3 ++ .../test in subpackage of safeds.sdstest | 3 ++ 11 files changed, 58 insertions(+), 15 deletions(-) create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe elsewhere.sdspipe create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in safeds.sdspipe create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in subpackage of safeds.sdspipe create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub elsewhere.sdsstub create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in safeds.sdsstub create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in subpackage of safeds.sdsstub create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/test elsewhere.sdstest create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in safeds.sdstest create mode 100644 tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in subpackage of safeds.sdstest diff --git a/src/language/validation/other/modules.ts b/src/language/validation/other/modules.ts index 2dcc94590..079cf379c 100644 --- a/src/language/validation/other/modules.ts +++ b/src/language/validation/other/modules.ts @@ -2,22 +2,10 @@ import { ValidationAcceptor } from 'langium'; import { isSdsDeclaration, isSdsPipeline, isSdsSegment, SdsDeclaration, SdsModule } from '../../generated/ast.js'; import { isInPipelineFile, isInStubFile } from '../../helpers/fileExtensions.js'; -export const CODE_MODULE_MISSING_PACKAGE = 'module/missing-package'; export const CODE_MODULE_FORBIDDEN_IN_PIPELINE_FILE = 'module/forbidden-in-pipeline-file'; export const CODE_MODULE_FORBIDDEN_IN_STUB_FILE = 'module/forbidden-in-stub-file'; - -export const moduleWithDeclarationsMustStatePackage = (node: SdsModule, accept: ValidationAcceptor): void => { - if (!node.name) { - const declarations = node.members.filter(isSdsDeclaration); - if (declarations.length > 0) { - accept('error', 'A module with declarations must state its package.', { - node: declarations[0], - property: 'name', - code: CODE_MODULE_MISSING_PACKAGE, - }); - } - } -}; +export const CODE_MODULE_MISSING_PACKAGE = 'module/missing-package'; +export const CODE_MODULE_PIPELINE_FILE_IN_SAFEDS_PACKAGE = 'module/pipeline-file-in-safeds-package'; export const moduleDeclarationsMustMatchFileKind = (node: SdsModule, accept: ValidationAcceptor): void => { const declarations = node.members.filter(isSdsDeclaration); @@ -52,3 +40,26 @@ export const declarationIsAllowedInPipelineFile = (declaration: SdsDeclaration): export const declarationIsAllowedInStubFile = (declaration: SdsDeclaration): boolean => { return !isSdsPipeline(declaration) && !isSdsSegment(declaration); }; + +export const moduleWithDeclarationsMustStatePackage = (node: SdsModule, accept: ValidationAcceptor): void => { + if (!node.name) { + const declarations = node.members.filter(isSdsDeclaration); + if (declarations.length > 0) { + accept('error', 'A module with declarations must state its package.', { + node: declarations[0], + property: 'name', + code: CODE_MODULE_MISSING_PACKAGE, + }); + } + } +}; + +export const pipelineFileMustNotBeInSafedsPackage = (node: SdsModule, accept: ValidationAcceptor): void => { + if (isInPipelineFile(node) && node.name?.startsWith('safeds')) { + accept('error', "A pipeline file must not be in a 'safeds' package.", { + node, + property: 'name', + code: CODE_MODULE_PIPELINE_FILE_IN_SAFEDS_PACKAGE, + }); + } +}; diff --git a/src/language/validation/safe-ds-validator.ts b/src/language/validation/safe-ds-validator.ts index a9ef8c3cc..bf67e2ee4 100644 --- a/src/language/validation/safe-ds-validator.ts +++ b/src/language/validation/safe-ds-validator.ts @@ -50,7 +50,11 @@ import { parameterMustHaveTypeHint, resultMustHaveTypeHint, } from './types.js'; -import { moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage } from './other/modules.js'; +import { + moduleDeclarationsMustMatchFileKind, + moduleWithDeclarationsMustStatePackage, + pipelineFileMustNotBeInSafedsPackage, +} from './other/modules.js'; import { typeParameterConstraintLeftOperandMustBeOwnTypeParameter } from './other/declarations/typeParameterConstraints.js'; import { parameterListMustNotHaveRequiredParametersAfterOptionalParameters } from './other/declarations/parameterLists.js'; import { unionTypeMustHaveTypes, unionTypeShouldNotHaveDuplicateTypes } from './other/types/unionTypes.js'; @@ -228,6 +232,7 @@ export const registerValidationChecks = function (services: SafeDsServices) { moduleMemberMustHaveNameThatIsUniqueInPackage(services), moduleMustContainUniqueNames, moduleWithDeclarationsMustStatePackage, + pipelineFileMustNotBeInSafedsPackage, pythonModuleShouldDifferFromSafeDsPackage(services), ], SdsNamedType: [ diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe elsewhere.sdspipe b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe elsewhere.sdspipe new file mode 100644 index 000000000..f193b218b --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe elsewhere.sdspipe @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »tests.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in safeds.sdspipe b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in safeds.sdspipe new file mode 100644 index 000000000..01bbbb021 --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in safeds.sdspipe @@ -0,0 +1,3 @@ +// $TEST$ error "A pipeline file must not be in a 'safeds' package." + +package »safeds« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in subpackage of safeds.sdspipe b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in subpackage of safeds.sdspipe new file mode 100644 index 000000000..84d64908d --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/pipe in subpackage of safeds.sdspipe @@ -0,0 +1,3 @@ +// $TEST$ error "A pipeline file must not be in a 'safeds' package." + +package »safeds.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub elsewhere.sdsstub b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub elsewhere.sdsstub new file mode 100644 index 000000000..f193b218b --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub elsewhere.sdsstub @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »tests.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in safeds.sdsstub b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in safeds.sdsstub new file mode 100644 index 000000000..07eecbb35 --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in safeds.sdsstub @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »safeds« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in subpackage of safeds.sdsstub b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in subpackage of safeds.sdsstub new file mode 100644 index 000000000..0724b7d84 --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/stub in subpackage of safeds.sdsstub @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »safeds.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test elsewhere.sdstest b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test elsewhere.sdstest new file mode 100644 index 000000000..f193b218b --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test elsewhere.sdstest @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »tests.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in safeds.sdstest b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in safeds.sdstest new file mode 100644 index 000000000..07eecbb35 --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in safeds.sdstest @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »safeds« diff --git a/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in subpackage of safeds.sdstest b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in subpackage of safeds.sdstest new file mode 100644 index 000000000..0724b7d84 --- /dev/null +++ b/tests/resources/validation/other/modules/pipeline files must not be in safeds package/test in subpackage of safeds.sdstest @@ -0,0 +1,3 @@ +// $TEST$ no error "A pipeline file must not be in a 'safeds' package." + +package »safeds.validation.other.modules.pipelineFilesMustNotBeInSafedsPackage« From c7c4becfeccbb5eeec81a0bb2517593a283e38a6 Mon Sep 17 00:00:00 2001 From: Lars Reimann Date: Sun, 22 Oct 2023 19:14:59 +0200 Subject: [PATCH 2/2] refactor: minor change --- src/language/validation/other/modules.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/language/validation/other/modules.ts b/src/language/validation/other/modules.ts index 079cf379c..6de62f1e3 100644 --- a/src/language/validation/other/modules.ts +++ b/src/language/validation/other/modules.ts @@ -1,6 +1,7 @@ import { ValidationAcceptor } from 'langium'; import { isSdsDeclaration, isSdsPipeline, isSdsSegment, SdsDeclaration, SdsModule } from '../../generated/ast.js'; import { isInPipelineFile, isInStubFile } from '../../helpers/fileExtensions.js'; +import { getModuleMembers } from '../../helpers/nodeProperties.js'; export const CODE_MODULE_FORBIDDEN_IN_PIPELINE_FILE = 'module/forbidden-in-pipeline-file'; export const CODE_MODULE_FORBIDDEN_IN_STUB_FILE = 'module/forbidden-in-stub-file'; @@ -43,10 +44,10 @@ export const declarationIsAllowedInStubFile = (declaration: SdsDeclaration): boo export const moduleWithDeclarationsMustStatePackage = (node: SdsModule, accept: ValidationAcceptor): void => { if (!node.name) { - const declarations = node.members.filter(isSdsDeclaration); - if (declarations.length > 0) { + const members = getModuleMembers(node); + if (members.length > 0) { accept('error', 'A module with declarations must state its package.', { - node: declarations[0], + node: members[0], property: 'name', code: CODE_MODULE_MISSING_PACKAGE, });