Skip to content

ArrayPrimitive rule: report variable/receiver types and factory methods - #3009

Merged
arturbosch merged 9 commits into
detekt:masterfrom
t-kameyama:issue_1245
Aug 25, 2020
Merged

arturbosch merged 9 commits into
detekt:masterfrom
t-kameyama:issue_1245

Conversation

@t-kameyama

Copy link
Copy Markdown
Contributor

Fixes #1245

@codecov

codecov Bot commented Aug 24, 2020 •

Copy link
Copy Markdown

Codecov Report

Merging #3009 into master will decrease coverage by 0.03%.
The diff coverage is 81.81%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3009      +/-   ##
============================================
- Coverage     79.70%   79.67%   -0.04%     
- Complexity     2504     2509       +5     
============================================
  Files           427      427              
  Lines          7543     7541       -2     
  Branches       1424     1430       +6     
============================================
- Hits           6012     6008       -4     
  Misses          773      773              
- Partials        758      760       +2     
Impacted Files Coverage Δ Complexity Δ
...urbosch/detekt/rules/performance/ArrayPrimitive.kt 74.07% <81.81%> (-8.69%) 15.00 <10.00> (+5.00) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 923b3b5...d57e8dd. Read the comment docs.

describe("variable type") {
it("is Array<Primitive>") {
val code = "val foo: Array<Int>? = null"
assertThat(subject.compileAndLint(code)).hasSize(1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(subject.compileAndLint(code)).hasSize(1)
assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)

describe("receiver type") {
it("is Array<Primitive>") {
val code = "fun Array<Long>.foo() { println(this) }"
assertThat(subject.compileAndLint(code)).hasSize(1)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(subject.compileAndLint(code)).hasSize(1)
assertThat(subject.compileAndLintWithContext(env, code)).hasSize(1)


it("is intArrayOf()") {
val code = "fun test(x: Int) = intArrayOf(x)"
assertThat(subject.compileAndLint(code)).isEmpty()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assertThat(subject.compileAndLint(code)).isEmpty()
assertThat(subject.compileAndLintWithContext(env, code)).isEmpty()

class ArrayPrimitive(config: Config = Config.empty) : Rule(config) {
companion object {
private val primitiveTypes = PrimitiveType.NUMBER_TYPES.map { it.typeName.asString() }
private val arrayOfFunctionFqName = FqName("kotlin.arrayOf")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Follow-up for the future: the same behavior should probably be flagged also for emptyArray()

@@ -35,44 +41,45 @@ import org.jetbrains.kotlin.psi.psiUtil.collectDescendantsOfType
* @active since v1.2.0

@cortinico cortinico Aug 24, 2020 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @active since v1.2.0
* @active since v1.2.0
* @requiresTypeResolution
*/

@cortinico

Copy link
Copy Markdown
Member

Side note: this is a rule that transition from NOT requiring to requiring type resolution. This should probably be noted in the README @arturbosch ?

@arturbosch arturbosch added this to the 1.12.0 milestone Aug 24, 2020
@arturbosch arturbosch added the migration Marker to add a migration step in the changelog label Aug 24, 2020
@t-kameyama
t-kameyama requested a review from cortinico August 25, 2020 01:05
* fun returningFunction(): DoubleArray { }
* </compliant>
*
* @active since v1.2.0

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not disable this rule.

@cortinico cortinico Aug 25, 2020 •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, I picked the wrong line for the suggestion. Updated

@arturbosch
arturbosch merged commit e036abe into detekt:master Aug 25, 2020
@t-kameyama
t-kameyama deleted the issue_1245 branch August 25, 2020 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

migration Marker to add a migration step in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rule: Prefer IntArray instead of Array<Int> to escape boxing

5 participants