Refactor ObjectPropertyNaming #1026
Conversation
| } | ||
| } | ||
|
|
||
| private fun doesNotMatchPattern(element: KtVariableDeclaration, pattern: Regex) = | ||
| !element.identifierName().matches(pattern) | ||
| private fun handleProperty(property: KtProperty) { |
| message = "Private object property names should match the pattern: $privatePropertyPattern")) | ||
| } | ||
| } else { | ||
| if (!property.identifierName().matches(propertyPattern)) { |
There was a problem hiding this comment.
I literally copied this code from TopLevelPropertyNaming. I'm happy to make this change, but I'll probably make it there too to keep it consistent.
| @@ -87,7 +88,6 @@ class ObjectPropertyNamingSpec : SubjectSpek<ObjectPropertyNaming>({ | |||
| val code = compileContentForTest(""" | |||
| object O { | |||
| val _nAme = "Artur" | |||
| private val _name = "Artur" | |||
| } | |||
| """) | |||
| assertThat(subject.lint(code)).hasSize(2) | |||
There was a problem hiding this comment.
Why does this test snippet report 2 violations?
It should only report 1 for the variable _nAme.
I suppose there's is an error in the rule implementation.
There was a problem hiding this comment.
Hmm. I changed the test to 1, and it still passed. I'm not convinced the test is actually running 😕
This adds a privatePropertyPattern and much more closely matches the code in TopLevelPropertyNaming.
| } | ||
| """) | ||
| assertThat(subject.lint(code)).hasSize(2) | ||
| assertThat(subject.lint(code)).hasSize(1) |
There was a problem hiding this comment.
@scottkennedy what did you change to only report 1 violation?
There was a problem hiding this comment.
Because there's only one property in the object now. I believe 1 is correct.
However, it wasn't failing the test when it was 2, and it's not failing now that it's 1, so I don't think the test is working.
There was a problem hiding this comment.
I checked out your branch on my machine. Running the tests in IntelliJ does work. However it doesn't when using the CLI.
This adds a privatePropertyPattern and much more closely matches the
code in TopLevelPropertyNaming. They may be able to share a base class (or be merged into one) in the future.