Backports from 3de4c3b26c1 - Fixes #105 #106
Conversation
* Added `findGetterDescriptionForField(FieldDescription base)` to check against all known prefixes for methods that clash * Extended test case for FieldPropertyClash to include predicate case
|
is that because of line endings ? I use linux, that kind of errors are not easy to detect. |
|
Some are,
|
| this.superType = superType; | ||
| } | ||
|
|
||
| public GetterDescription findGetterDescriptionForField(FieldDescription base) { |
There was a problem hiding this comment.
can you add unit tests for this in FieldDescriptionTest ?
There was a problem hiding this comment.
Wouldn't it make more sense in ClassDescriptionTest?
Edit: It would, but that class doesn't exist... 😅
| field.getTypeDescription(), | ||
| Collections.<TypeName> emptyList()))) { | ||
|
|
||
| if (classDescription.findGetterDescriptionForField(field) != null) { |
There was a problem hiding this comment.
rename findGetterDescriptionForField to hasGetterForField and simply return a boolean
There was a problem hiding this comment.
I agree with the name. For return type, I think it could be useful later to have it returning the value. If we were on Java 8 I'd rather return an Optional<>, though. Thought?s
There was a problem hiding this comment.
Additionally, I've realized that there is a very real possibility of multiple. So either:
- return a
Set<>ofGetters - Boolean on first found
I'm okay with either.
| return getterDesc; | ||
| } | ||
| } | ||
| } else { |
There was a problem hiding this comment.
I think the else can be removed and replaced by this one line (assuming we return a boolean):
return fieldMap.containsKey("get" + capName)
|
Sure thing, I'll get to it later today! |
* Addresses comments from @joel-costigliola in assertj#106 * Broke method into two: one for presence, one for getting the values * Added many more tests
2 participants
Backport of 3de4c3b.
findGetterDescriptionForField(FieldDescription base)to check against all known prefixes for methods that clashFun fact: The current master branch fails on windows, I had to use a vagrant machine to test this. 😢
Fixes #105.