Deprecate Location.locationString #2084
Conversation
There was a problem hiding this comment.
Thanks for refactoring this assert method!
This makes the test more maintainable and easier to read.
Cool!
To further improve the test we should consider having a test for each rule, which asserts the reported element at least once.
Of course this should be in a separate PR.
This is however a very good first step.
|
What do you think about a PR to talk about how to test a rule? The tests right now are SO different between them... There's no need to refactor all of them now but at least we should know how we want them. |
|
@BraisGabin we have an existing issues regarding the rigorous case files (#1089).
It might be worth to create an issue for that. Docs would also be nice. Maybe that's a thing for the contributing guide in the |
|
@schalkms I highly agree that we should a paragraph about the prefered way of testing detekt rules 👍 |
* Use hasSourceLocations instead of hasLocationString * Deprecate locationString * Generate documentation
* Use hasSourceLocations instead of hasLocationString * Deprecate locationString * Generate documentation
This PR is a continuation of #2014
This code is a bit strange: https://github.com/arturbosch/detekt/pull/2083/files#diff-3d9aab0c44e865c3fbae3f5f7a8edc8dR67 for that reason I research a bit about
Location.locationStringand it's not used in the production code. The only uses ar in the test code.In this PR I refactor the tests to don't use that value and I add the
@Deprecateannotation to that property.