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: port remaining validation infos that don't need partial evaluation #607

Merged
merged 4 commits into from
Oct 6, 2023
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
4 changes: 4 additions & 0 deletions src/language/helpers/nodeProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,10 @@ export const isNamedTypeArgument = (node: SdsTypeArgument): boolean => {
return Boolean(node.typeParameter);
};

export const isRequiredParameter = (node: SdsParameter): boolean => {
return !node.defaultValue && !node.isVariadic;
};

export const isStatic = (node: SdsClassMember): boolean => {
if (isSdsClass(node) || isSdsEnum(node)) {
return true;
Expand Down
8 changes: 8 additions & 0 deletions src/language/validation/safe-ds-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@ import {
segmentMustContainUniqueNames,
} from './names.js';
import {
annotationCallArgumentListShouldBeNeeded,
annotationParameterListShouldNotBeEmpty,
assignmentShouldHaveMoreThanWildcardsAsAssignees,
callArgumentListShouldBeNeeded,
classBodyShouldNotBeEmpty,
constraintListShouldNotBeEmpty,
enumBodyShouldNotBeEmpty,
enumVariantParameterListShouldNotBeEmpty,
functionResultListShouldNotBeEmpty,
memberAccessNullSafetyShouldBeNeeded,
namedTypeTypeArgumentListShouldBeNeeded,
segmentResultListShouldNotBeEmpty,
typeParameterListShouldNotBeEmpty,
unionTypeShouldNotHaveASingularTypeArgument,
Expand Down Expand Up @@ -52,9 +56,11 @@ export const registerValidationChecks = function (services: SafeDsServices) {
const checks: ValidationChecks<SafeDsAstType> = {
SdsAssignment: [assignmentShouldHaveMoreThanWildcardsAsAssignees],
SdsAnnotation: [annotationMustContainUniqueNames, annotationParameterListShouldNotBeEmpty],
SdsAnnotationCall: [annotationCallArgumentListShouldBeNeeded],
SdsArgumentList: [argumentListMustNotHavePositionalArgumentsAfterNamedArguments],
SdsAttribute: [attributeMustHaveTypeHint],
SdsBlockLambda: [blockLambdaMustContainUniqueNames],
SdsCall: [callArgumentListShouldBeNeeded(services)],
SdsCallableType: [callableTypeMustContainUniqueNames, callableTypeMustNotHaveOptionalParameters],
SdsClass: [classMustContainUniqueNames],
SdsClassBody: [classBodyShouldNotBeEmpty],
Expand All @@ -65,7 +71,9 @@ export const registerValidationChecks = function (services: SafeDsServices) {
SdsEnumVariant: [enumVariantMustContainUniqueNames, enumVariantParameterListShouldNotBeEmpty],
SdsExpressionLambda: [expressionLambdaMustContainUniqueNames],
SdsFunction: [functionMustContainUniqueNames, functionResultListShouldNotBeEmpty],
SdsMemberAccess: [memberAccessNullSafetyShouldBeNeeded(services)],
SdsModule: [moduleDeclarationsMustMatchFileKind, moduleWithDeclarationsMustStatePackage],
SdsNamedType: [namedTypeTypeArgumentListShouldBeNeeded],
SdsParameter: [parameterMustHaveTypeHint, parameterMustNotBeVariadicAndOptional],
SdsParameterList: [
parameterListMustNotHaveOptionalAndVariadicParameters,
Expand Down
109 changes: 106 additions & 3 deletions src/language/validation/style.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,88 @@
import {
isSdsEnumVariant,
isSdsWildcard,
SdsAnnotation,
SdsAnnotationCall,
SdsAssignment,
SdsCall,
SdsClassBody,
SdsConstraintList,
SdsEnumBody,
SdsEnumVariant,
SdsFunction,
SdsMemberAccess,
SdsNamedType,
SdsSegment,
SdsTypeParameterList,
SdsUnionType,
} from '../generated/ast.js';
import { ValidationAcceptor } from 'langium';
import { isEmpty } from 'radash';
import { isRequiredParameter, parametersOrEmpty, typeParametersOrEmpty } from '../helpers/nodeProperties.js';
import { SafeDsServices } from '../safe-ds-module.js';
import { UnknownType } from '../typing/model.js';

export const CODE_STYLE_UNNECESSARY_ASSIGNMENT = 'style/unnecessary-assignment';
export const CODE_STYLE_UNNECESSARY_ARGUMENT_LIST = 'style/unnecessary-argument-list';
export const CODE_STYLE_UNNECESSARY_BODY = 'style/unnecessary-body';
export const CODE_STYLE_UNNECESSARY_CONSTRAINT_LIST = 'style/unnecessary-constraint-list';
export const CODE_STYLE_UNNECESSARY_ELVIS_OPERATOR = 'style/unnecessary-elvis-operator';
export const CODE_STYLE_UNNECESSARY_SAFE_ACCESS = 'style/unnecessary-safe-access';
export const CODE_STYLE_UNNECESSARY_PARAMETER_LIST = 'style/unnecessary-parameter-list';
export const CODE_STYLE_UNNECESSARY_RESULT_LIST = 'style/unnecessary-result-list';
export const CODE_STYLE_UNNECESSARY_SAFE_ACCESS = 'style/unnecessary-safe-access';
export const CODE_STYLE_UNNECESSARY_TYPE_ARGUMENT_LIST = 'style/unnecessary-type-argument-list';
export const CODE_STYLE_UNNECESSARY_TYPE_PARAMETER_LIST = 'style/unnecessary-type-parameter-list';
export const CODE_STYLE_UNNECESSARY_UNION_TYPE = 'style/unnecessary-union-type';

// -----------------------------------------------------------------------------
// Unnecessary assignment
// Unnecessary argument lists
// -----------------------------------------------------------------------------

export const annotationCallArgumentListShouldBeNeeded = (node: SdsAnnotationCall, accept: ValidationAcceptor): void => {
const argumentList = node.argumentList;
if (!argumentList || !isEmpty(argumentList.arguments)) {
// If there are arguments, they are either needed or erroneous (i.e. we already show an error)
return;
}

const annotation = node.annotation?.ref;
if (!annotation) {
return;
}

const hasRequiredParameters = parametersOrEmpty(annotation).some(isRequiredParameter);
if (!hasRequiredParameters) {
accept('info', 'This argument list can be removed.', {
node: argumentList,
code: CODE_STYLE_UNNECESSARY_ARGUMENT_LIST,
});
}
};

export const callArgumentListShouldBeNeeded =
(services: SafeDsServices) =>
(node: SdsCall, accept: ValidationAcceptor): void => {
const argumentList = node.argumentList;
if (!argumentList || !isEmpty(argumentList.arguments)) {
// If there are arguments, they are either needed or erroneous (i.e. we already show an error)
return;
}

const callable = services.helpers.NodeMapper.callToCallableOrUndefined(node);
if (!isSdsEnumVariant(callable)) {
return;
}

if (isEmpty(parametersOrEmpty(callable))) {
accept('info', 'This argument list can be removed.', {
node: argumentList,
code: CODE_STYLE_UNNECESSARY_ARGUMENT_LIST,
});
}
};

// -----------------------------------------------------------------------------
// Unnecessary assignments
// -----------------------------------------------------------------------------

export const assignmentShouldHaveMoreThanWildcardsAsAssignees = (
Expand Down Expand Up @@ -66,7 +121,7 @@ export const enumBodyShouldNotBeEmpty = (node: SdsEnumBody, accept: ValidationAc
};

// -----------------------------------------------------------------------------
// Unnecessary constraint list
// Unnecessary constraint lists
// -----------------------------------------------------------------------------

export const constraintListShouldNotBeEmpty = (node: SdsConstraintList, accept: ValidationAcceptor) => {
Expand Down Expand Up @@ -126,6 +181,54 @@ export const segmentResultListShouldNotBeEmpty = (node: SdsSegment, accept: Vali
}
};

// -----------------------------------------------------------------------------
// Unnecessary safe access
// -----------------------------------------------------------------------------

export const memberAccessNullSafetyShouldBeNeeded =
(services: SafeDsServices) =>
(node: SdsMemberAccess, accept: ValidationAcceptor): void => {
if (!node.isNullSafe) {
return;
}

const receiverType = services.types.TypeComputer.computeType(node.receiver);
if (receiverType === UnknownType) {
return;
}

if (!receiverType.isNullable) {
accept('info', 'The receiver is never null, so the safe access is unnecessary.', {
node,
code: CODE_STYLE_UNNECESSARY_SAFE_ACCESS,
});
}
};

// -----------------------------------------------------------------------------
// Unnecessary type argument lists
// -----------------------------------------------------------------------------

export const namedTypeTypeArgumentListShouldBeNeeded = (node: SdsNamedType, accept: ValidationAcceptor): void => {
const typeArgumentList = node.typeArgumentList;
if (!typeArgumentList || !isEmpty(typeArgumentList.typeArguments)) {
// If there are type arguments, they are either needed or erroneous (i.e. we already show an error)
return;
}

const namedTypeDeclaration = node.declaration?.ref;
if (!namedTypeDeclaration) {
return;
}

if (isEmpty(typeParametersOrEmpty(namedTypeDeclaration))) {
accept('info', 'This type argument list can be removed.', {
node: typeArgumentList,
code: CODE_STYLE_UNNECESSARY_ARGUMENT_LIST,
});
}
};

// -----------------------------------------------------------------------------
// Unnecessary type parameter lists
// -----------------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package tests.validation.style.unnecessaryArgumentListInAnnotationCall

@Repeatable
annotation AnnotationWithoutParameterList

@Repeatable
annotation AnnotationWithEmptyParameterList()

@Repeatable
annotation AnnotationWithoutRequiredParameters(a: Int = 0, vararg b: Int)

@Repeatable
annotation AnnotationWithRequiredParameters(a: Int)

// $TEST$ info "This argument list can be removed."
@AnnotationWithoutParameterList»()«
// $TEST$ no info "This argument list can be removed."
@AnnotationWithoutParameterList»(1)«
// $TEST$ info "This argument list can be removed."
@AnnotationWithEmptyParameterList»()«
// $TEST$ no info "This argument list can be removed."
@AnnotationWithEmptyParameterList»(1)«
// $TEST$ info "This argument list can be removed."
@AnnotationWithoutRequiredParameters»()«
// $TEST$ no info "This argument list can be removed."
@AnnotationWithoutRequiredParameters»(1)«
// $TEST$ no info "This argument list can be removed."
@AnnotationWithRequiredParameters»()«
// $TEST$ no info "This argument list can be removed."
@AnnotationWithRequiredParameters»(1)«
// $TEST$ no info "This argument list can be removed."
@Unresolved»()«
// $TEST$ no info "This argument list can be removed."
@Unresolved»(1)«
class MyClass
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package tests.validation.style.unnecessaryArgumentListInCall

annotation MyAnnotation

class MyClass()

enum MyEnum {
EnumVariantWithoutParameterList
EnumVariantWithEmptyParameterList()
EnumVariantWithoutRequiredParameters(a: Int = 0, vararg b: Int)
EnumVariantWithRequiredParameters(a: Int)
}

fun myFunction()

segment mySegment1() {}

segment mySegment2(
callableType: () -> (),
) {
val blockLambda = () {};
val expressionLambda = () -> 0;

// $TEST$ no info "This argument list can be removed."
MyAnnotation»()«;

// $TEST$ no info "This argument list can be removed."
MyClass»()«;

// $TEST$ info "This argument list can be removed."
MyEnum.EnumVariantWithoutParameterList»()«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithoutParameterList»(1)«;
// $TEST$ info "This argument list can be removed."
MyEnum.EnumVariantWithEmptyParameterList»()«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithEmptyParameterList»(1)«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithoutRequiredParameters»()«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithoutRequiredParameters»(1)«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithRequiredParameters»()«;
// $TEST$ no info "This argument list can be removed."
MyEnum.EnumVariantWithRequiredParameters»(1)«;
// $TEST$ no info "This argument list can be removed."
MyEnum.Unresolved»()«;
// $TEST$ no info "This argument list can be removed."
MyEnum.Unresolved»(1)«;

// $TEST$ no info "This argument list can be removed."
myFunction»()«;

// $TEST$ no info "This argument list can be removed."
mySegment1»()«;

// $TEST$ no info "This argument list can be removed."
callableType»()«;

// $TEST$ no info "This argument list can be removed."
blockLambda»()«;

// $TEST$ no info "This argument list can be removed."
expressionLambda»()«;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package tests.validation.style.unnecessarySafeAccess

pipeline test {
// $TEST$ info "The receiver is never null, so the safe access is unnecessary."
»1?.toString«();
// $TEST$ no info "The receiver is never null, so the safe access is unnecessary."
»null?.toString«();
// $TEST$ no info "The receiver is never null, so the safe access is unnecessary."
»unresolved?.toString«();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package tests.validation.style.unnecessaryTypeArgumentList

class ClassWithoutTypeParameterList
class ClassWithEmptyTypeParameterList<>
class ClassWithTypeParameters<T>

enum Enum {
EnumVariantWithoutTypeParameterList
EnumVariantWithEmptyTypeParameterList<>
VariantWithTypeParameters<T>
}

fun myFunction(
// $TEST$ info "This type argument list can be removed."
a1: ClassWithoutTypeParameterList»<>«,
// $TEST$ no info "This type argument list can be removed."
a2: ClassWithoutTypeParameterList»<Int>«,
// $TEST$ info "This type argument list can be removed."
a3: ClassWithEmptyTypeParameterList»<>«,
// $TEST$ no info "This type argument list can be removed."
a4: ClassWithEmptyTypeParameterList»<Int>«,
// $TEST$ no info "This type argument list can be removed."
a5: ClassWithTypeParameters»<>«,
// $TEST$ no info "This type argument list can be removed."
a6: ClassWithTypeParameters»<Int>«,

// $TEST$ info "This type argument list can be removed."
b1: Enum.EnumVariantWithoutTypeParameterList»<>«,
// $TEST$ no info "This type argument list can be removed."
b2: Enum.EnumVariantWithoutTypeParameterList»<Int>«,
// $TEST$ info "This type argument list can be removed."
b3: Enum.EnumVariantWithEmptyTypeParameterList»<>«,
// $TEST$ no info "This type argument list can be removed."
b4: Enum.EnumVariantWithEmptyTypeParameterList»<Int>«,
// $TEST$ no info "This type argument list can be removed."
b5: Enum.VariantWithTypeParameters»<>«,
// $TEST$ no info "This type argument list can be removed."
b6: Enum.VariantWithTypeParameters»<Int>«,

// $TEST$ info "This type argument list can be removed."
c1: Enum»<>«,
// $TEST$ no info "This type argument list can be removed."
c2: Enum»<Int>«,

// $TEST$ no info "This type argument list can be removed."
d1: Unresolved»<>«,
// $TEST$ no info "This type argument list can be removed."
d2: Unresolved»<Int>«,
)