Rewrite OpticsProcessor and write some tests for transforming #788
Conversation
build.gradle
Outdated
| maxParallelForks = Runtime.runtime.availableProcessors() | ||
| } | ||
|
|
||
| // build.dependsOn ':detekt' |
| normalizedTargets.forEach { target -> | ||
|
|
||
| when (target) { | ||
| OpticsTarget.LENS -> annotatedLenses.addAll(evalAnnotatedLensElement(element).singleToList()) |
There was a problem hiding this comment.
Nitpick.
Unwrapping null with let, adding addNonNull or using knowError instead of logE seems clearer than singleToList imo.
There was a problem hiding this comment.
I haven't realized there is addIfNotNull method
| } | ||
| } | ||
|
|
||
| private fun evalAnnotatedDslElement(element: Element): AnnotatedOptic? = when (element.getClassType()) { |
There was a problem hiding this comment.
evalAnnotatedOptionalElement, evalAnnotatedOptionalElement and evalAnnotatedDslElement is same method with only difference error message.
|
|
||
| temp.listFiles().size shouldBe 1 | ||
| temp.listFiles().size shouldBe 1 |
There was a problem hiding this comment.
Shouldn't this be configurable? This doesn't work for i.e. DSL.
There was a problem hiding this comment.
It will be in next PR where I'll add DSL tests.
| logE("Only data classes can have DSL target", element) | ||
| emptyList() | ||
| } else { | ||
| targets.toSet().plus(listOf(OpticsTarget.LENS, OpticsTarget.OPTIONAL)).toList() |
There was a problem hiding this comment.
targets.toList() + listOf(OpticsTarget.LENS, OpticsTarget.OPTIONAL)
There was a problem hiding this comment.
This also removes the need for LensesFileGenerator(annotatedBounded, generatedDir).generate()and OptionalFileGenerator(annotatedBounded, generatedDir).generate()
| return@forEach | ||
| } | ||
|
|
||
| val normalizedTargets = if (targets.isEmpty()) { |
There was a problem hiding this comment.
This snippet could drastically be "simplified" using when.
Codecov Report
@@ Coverage Diff @@
## master #788 +/- ##
============================================
+ Coverage 41.01% 43.44% +2.43%
- Complexity 510 582 +72
============================================
Files 277 282 +5
Lines 7156 7211 +55
Branches 815 812 -3
============================================
+ Hits 2935 3133 +198
+ Misses 3961 3791 -170
- Partials 260 287 +27
Continue to review full report at Codecov.
|
|
@nomisRev I think it's ready |
|
@nomisRev I don't think so. There is nothing about spaces in docs: https://github.com/arturbosch/detekt/blob/master/docs/pages/documentation/style.md EDIT: But on the other hand there is this: detekt/detekt@9ba1815 EDIT2: Also, build isn't "failing". My PR just have coverage lower than project's overall. But still it'll increase coverage. |
|
@jereksel We have it configured here but it doesn't seem to work. Or at least not as I expect it to. I'll just run formatter locally and merge. Awesome work :) EDIT: Ah I thought merging was blocked when not all checks passed. Which I translated to "failed". |
I've remove silent failing when annotation is incorrect - now it throws error. I've also changed default value of array in optics to empty one.