Skip to content

Commit

Permalink
Create rule S7058: Single cascade shouldn't be used
Browse files Browse the repository at this point in the history
  • Loading branch information
leveretka committed Sep 3, 2024
1 parent 3d6e106 commit f9c89de
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 0 deletions.
23 changes: 23 additions & 0 deletions rules/S7058/dart/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"title": "Single cascade shouldn't be used",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "2min"
},
"tags": [
],
"defaultSeverity": "Major",
"ruleSpecification": "RSPEC-7058",
"sqKey": "S7058",
"scope": "All",
"defaultQualityProfiles": ["Sonar way"],
"quickfix": "unknown",
"code": {
"impacts": {
"MAINTAINABILITY": "LOW"
},
"attribute": "CONVENTIONAL"
}
}
97 changes: 97 additions & 0 deletions rules/S7058/dart/rule.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
== Why is this an issue?

Cascade notation in dart is a convenient way of chaining several operations on the same object, while also having access to the instance methods and fields of the cascading object. With cascades you can write such code:

[source,dart]
----
var person = Person()
..firstName = 'John'
..lastName = 'Smith'
..age = 50;
----

which will be the equivalent of:

[source,dart]
----
var person = Person();
person.firstName = 'John';
person.lastName = 'Smith';
person.age = 50;
----

We can think of the cascade notation as a concise implementation of a builder pattern. Moreover, there's a null-safe version of it `?..` that will only access members if the cascading object isn't `null`.

While such notations looks appealing it doesn't really make sense to use for a single cascade section. For example, `person..read();` seems to be an overkill. In this case usual `person.read()` is much more clear. Sometimes it may also become a source of bugs. Looks at this example:

[source,dart]
----
var person = Person()..getName(); // here the instance of Person will be returned
var name = Person().getName(); // here the name will be returned
----

So be careful, when use cascading especially in combination with `var`. And always make sure the expected value is returned.

The other side of this notation os that it might be confused with a range operator `..` from other languages (like Kotlin). So using it without necessity will make code less readable and as a result less maintainable.

== How to fix it
Just replace cascade notation (`..`) with a single dot (`.`) or make sure you assign the constructed object.

=== Code examples

==== Noncompliant code example

[source,dart,diff-id=1,diff-type=noncompliant]
----
Person()..doSomething();
----

==== Compliant solution

[source,dart,diff-id=1,diff-type=compliant]
----
var person = Person()..doSomething(); // if we want to return the Person instance
----

==== Noncompliant code example

[source,dart,diff-id=2,diff-type=noncompliant]
----
Person()..doSomething();
----

==== Compliant solution

[source,dart,diff-id=2,diff-type=compliant]
----
Person().doSomething();
----

== Resources

=== Documentation

* Dart Docs - https://dart.dev/tools/linter-rules/avoid_single_cascade_in_expression_statements[Dart Linter rule - avoid_single_cascade_in_expression_statements]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

* Unnecessary cascade expression.

=== Highlighting

Cascade expression

'''
== Comments And Links
(visible only on this page)

endif::env-github,rspecator-view[]


2 changes: 2 additions & 0 deletions rules/S7058/metadata.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}

0 comments on commit f9c89de

Please sign in to comment.