Skip to content

Commit

Permalink
feat: port remaining validation infos that don't need partial evaluat…
Browse files Browse the repository at this point in the history 
…ion (#607)

Closes partially #543.

### Summary of Changes

Show an info if
* an argument list can be removed,
* a type argument list can be removed,
* a safe access can be removed.

---------

Co-authored-by: megalinter-bot <[email protected]>
  • Loading branch information
@lars-reimann @megalinter-bot
lars-reimann and megalinter-bot authored Oct 6, 2023
1 parent 4fd8d86 commit d53bda3
Show file tree
Hide file tree
Showing 7 changed files with 277 additions and 3 deletions.
4 changes: 4 additions & 0 deletions src/language/helpers/nodeProperties.ts
View file
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
View file
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
View file
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
35 changes: 35 additions & 0 deletions tests/resources/validation/style/unnecessary argument list in annotation call/main.sdstest
View file
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
65 changes: 65 additions & 0 deletions tests/resources/validation/style/unnecessary argument list in call/main.sdstest
View file
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»()«;
}
10 changes: 10 additions & 0 deletions tests/resources/validation/style/unnecessary safe access/main.sdstest
View file
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«();
}
49 changes: 49 additions & 0 deletions tests/resources/validation/style/unnecessary type argument list/main.sdstest
View file
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>«,
)

0 comments on commit d53bda3

Please sign in to comment.