Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: show error if own declaration has same name as core one #762

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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`;
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«