Skip to content

Commit

Permalink
feat: show error if own declaration has same name as core one (#762)
Browse files Browse the repository at this point in the history
Closes #760

### Summary of Changes

Core declarations like `Any` should never be shadowed by own
declarations. Thus, it's now an error to give own declarations the same
name as a core declaration.
  • Loading branch information
lars-reimann authored Nov 11, 2023
1 parent 36663ca commit 8cb2120
Show file tree
Hide file tree
Showing 8 changed files with 52 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export const BUILTINS_ROOT_PACKAGE = 'safeds';
export const BUILTINS_LANG_PACKAGE = `${BUILTINS_ROOT_PACKAGE}.lang`;
9 changes: 6 additions & 3 deletions packages/safe-ds-lang/src/language/validation/inheritance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
);
} else if (typeChecker.isAssignableTo(overriddenMemberType, ownMemberType)) {
// Prevents the info from showing when editing the builtin files
if (isInSafedsLangAnyClass(node)) {
if (isInSafedsLangAnyClass(services, node)) {
return;
}

Expand All @@ -54,9 +54,12 @@ export const classMemberMustMatchOverriddenMemberAndShouldBeNeeded = (services:
};
};

const isInSafedsLangAnyClass = (node: SdsClassMember): boolean => {
const isInSafedsLangAnyClass = (services: SafeDsServices, node: SdsClassMember): boolean => {
const containingClass = getContainerOfType(node, isSdsClass);
return isSdsClass(containingClass) && getQualifiedName(containingClass) === 'safeds.lang.Any';
return (
isSdsClass(containingClass) &&
getQualifiedName(containingClass) === getQualifiedName(services.builtins.Classes.Any)
);
};

export const classMustOnlyInheritASingleClass = (services: SafeDsServices) => {
Expand Down
33 changes: 32 additions & 1 deletion packages/safe-ds-lang/src/language/validation/names.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { AstNodeDescription, getDocument, ValidationAcceptor } from 'langium';
import { duplicatesBy } from '../../helpers/collectionUtils.js';
import { listBuiltinFiles } from '../builtins/fileFinder.js';
import { BUILTINS_ROOT_PACKAGE } from '../builtins/packageNames.js';
import { BUILTINS_LANG_PACKAGE, BUILTINS_ROOT_PACKAGE } from '../builtins/packageNames.js';
import {
isSdsQualifiedImport,
SdsAnnotation,
Expand Down Expand Up @@ -46,6 +46,7 @@ import { SafeDsServices } from '../safe-ds-module.js';
import { declarationIsAllowedInPipelineFile, declarationIsAllowedInStubFile } from './other/modules.js';

export const CODE_NAME_CODEGEN_PREFIX = 'name/codegen-prefix';
export const CODE_NAME_CORE_DECLARATION = 'name/core-declaration';
export const CODE_NAME_CASING = 'name/casing';
export const CODE_NAME_DUPLICATE = 'name/duplicate';

Expand All @@ -68,6 +69,36 @@ export const nameMustNotStartWithCodegenPrefix = (node: SdsDeclaration, accept:
}
};

// -----------------------------------------------------------------------------
// Core declaration
// -----------------------------------------------------------------------------

export const nameMustNotOccurOnCoreDeclaration = (services: SafeDsServices) => {
const packageManager = services.workspace.PackageManager;

return (node: SdsDeclaration, accept: ValidationAcceptor) => {
if (!node.name) {
/* c8 ignore next 2 */
return;
}

// Prevents the error from showing when editing the builtin files
const packageName = getPackageName(node);
if (packageName === BUILTINS_LANG_PACKAGE) {
return;
}

const coreDeclarations = packageManager.getDeclarationsInPackage(BUILTINS_LANG_PACKAGE);
if (coreDeclarations.some((it) => it.name === node.name)) {
accept('error', 'Names of core declarations must not be used for own declarations.', {
node,
property: 'name',
code: CODE_NAME_CORE_DECLARATION,
});
}
};
};

// -----------------------------------------------------------------------------
// Casing
// -----------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
functionMustContainUniqueNames,
moduleMemberMustHaveNameThatIsUniqueInPackage,
moduleMustContainUniqueNames,
nameMustNotOccurOnCoreDeclaration,
nameMustNotStartWithCodegenPrefix,
nameShouldHaveCorrectCasing,
pipelineMustContainUniqueNames,
Expand Down Expand Up @@ -231,6 +232,7 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsClassMember: [classMemberMustMatchOverriddenMemberAndShouldBeNeeded(services)],
SdsConstraintList: [constraintListsShouldBeUsedWithCaution, constraintListShouldNotBeEmpty],
SdsDeclaration: [
nameMustNotOccurOnCoreDeclaration(services),
nameMustNotStartWithCodegenPrefix,
nameShouldHaveCorrectCasing,
pythonNameShouldDifferFromSafeDsName(services),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
package safeds.lang

// $TEST$ no error "Names of core declarations must not be used for own declarations."
annotation »Number«
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package tests.validation.names.coreNames

// $TEST$ error "Names of core declarations must not be used for own declarations."
annotation »Any«

// $TEST$ no error "Names of core declarations must not be used for own declarations."
annotation »Any1«

0 comments on commit 8cb2120

Please sign in to comment.