Add a new rule MaxChainedCallsOnSameLine to limit the number of chained calls on placed on a single line. This works well alongside CascadingCallWrapping in #4979 to make long call chains more readable by wrapping them on new lines.
Fix a missed case of UnnecessaryApply where the apply contains a single field assignment but the result of the apply{} is unused. This pattern is actually used in 2/3 of the <noncompliant> examples and while it occurred in tests for false positives, no test actually checked that warnings were generated in the simple case.
* Report null-check returning 'Unit'
The rule will report code smell when null-check expression returns 'Unit'
* Not report guard statement with side effect ahead
Add a new rule CascadingCallWrapping which requires that if a chained call is placed on a newline then all subsequent calls must be as well, improving readability of long chains.
This rule is based on discussion #4890.
Checks for suppression of one or more rules that have been decided to
never be suppressed within a code base. For example, if we've set
MaximumLineLength to a value, every file must be compliant, and if there
is a special case where compliance is discouraged, we can simply exclude
this file within YML configuration.
This rule cannot prevent itself from being suppressed, but should serve
to discourage the over use of `@Suppress` annotations.
* Allow additionalJavaSourceRootPaths to be defined on @KotlinCoreEnvironmentTest
* update api
* Rename top level nested classes to WithDefaultSources and WithAdditionalJavaSources
* fix compilation error in java < 11
* fix upstream merge
* fix: do not attempt to compile snippet with java class from resource
* manually fix indentation of test snippets
Co-authored-by: Markus Schwarz <post@markus-schwarz.net>
Fix an issue where UselessCallOnNotNull would report issues when some of the argument types resolved as ErrorType. While it correctly handles cases where the BindingContext cannot resolve a type (the first test case), in cases where a call is made on an unknown type (the second test case, e.g. via takeIf{}) the BindingContext will report it as an ErrorType. Since ErrorType.isNullable() apparently returns false we need to make an exception to ensure the rule behaves as expected.
CanBeNonNullable reports that properties of parameterized types can be marked as non-nullable even when this is impossible, just because the parameterized type is itself nullable (i.e. does not inherit from a non-nullable type). Instead, the rule should only report cases where a property is explicitly marked nullable but does not need to be.
Add a new rule which enforces the recommendation of the Kotlin coding conventions to prefer `==` over `?:` when converting a `Boolean?` into a `Boolean`: https://kotlinlang.org/docs/coding-conventions.html#nullable-boolean-values-in-conditions. This is simple to implement and enforces consistency for the two essentially equivalent methods of coalescing nullable boolean values.
The coding conventions only specify this preference for usages "in a conditional statement" but the same rationale applies to any statement which converts a Boolean? to a Boolean, so I have implemented it for any situation. An alternative is to add a configuration setting to toggle application only in if statements.
Unfortunately, this rule requires type resolution since there are technically valid usages of e.g. `value ?: true` for cases where `value` is not a `Boolean?` but an `Any?`. Running without type resolution will not be able to distinguish between these valid usages and ones which could be converted to `== false`.
* UnnecessaryInnerClass: fix false positives labeled expression to outer class
Fix an issue where inner classes which reference their outer class via a labeled
expression would still be reported as unnecessary. This seems to be due to a
missing visitExpressionWithLabel() check which resolves the label name: if the
label is referencing a parent class, then the nested class must be `inner`.
I wasn't able to find a way to resolve the labeled expression as a ClassId to
match the existing class check from KtReferenceExpression, so I settled for
using the class name instead, which is easy to obtain. This means there's a fair
bit of duplication in overriden checkForOuterUsage() methods to check parent
classes which could probably be removed, but I didn't see a particularly clean
way to do so.
* UnnecessaryInnerClass: add test for double-nested inner classes using labeled expressions
Add a unit test for a bug in which safe qualified references to a nullable field
in the parent class (i.e. as `?.`) would not be marked as warnings in release
1.20.0. This issue has been inadvertently fixed by
https://github.com/detekt/detekt/pull/4738, but adding a unit test will guard
against future regressions.
I was able to confirm this test fails by checking out the 1.20.0 tag and running
the unit test; against that tag the fix was to override the
visitSafeQualifiedExpression() function with the same logic as
visitCallExpression().
* fix: add test case that fails if environment is not properly set up
* add reasoning for additional test case
Co-authored-by: Markus Schwarz <post@markus-schwarz.net>
* Simplify AnnotationExcluderSpec
* Improve tests
* Improve tests
* Fix FullQualifiedNameGuesser
* Allow List<Regex> inside AnnotationExcluder to give more options to the callers
* Test with and without type-solving
* Make AnnotationExcluder use the BindingContext when available
* AnnotationSuppressor works with Full Qualified names without type solving
* Address PR comments
* Update detekt-psi-utils/src/test/kotlin/io/github/detekt/psi/internal/FullQualifiedNameGuesserSpec.kt
Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>
* Support star imports
* Improve readability
Co-authored-by: M Schalk <30376729+schalkms@users.noreply.github.com>
* Fix false positive of UnncessaryInnerClass
* Exclude Nested annotation because junit test requires @Nested test class to be inner
* Fix test code snippet
