This repository has been archived by the owner on Feb 19, 2023. It is now read-only.
feat: add rule to notify on using a double space #65
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The common module will house any shared code for the other modules, such as shared Issue classes
Since issues in the common space will need to share their details with other issues in the other modules, add a new interface that allows any other issues to implement to store its basic info. In this case the MultipleSpaceIssue will implement the LintIssue to set the basic details such as priority and id, and then the MultipleSpaceIssue will be implemented by another class to set the Issue field
Add the initial issue object, and the detector file
Check through all the lines in a Gradle file to find any instances of " ", if it is found raise an error for that line
Using a regex is much less code and allows easily finding the exact location of where the issues occur
Currently the regex to find spaces will check for word characters, but it should be checking for anything that is not a white space to ensure that something like `. words` is also picked up
Add a test for the failure case and a test for the success case
Add a matching issue detector for multiple spaces for the xml module as well, currently this is a duplicated file, it will be worth trying to share the same detector across both modules
Add a base detector so that both of the MultipleSpaceDetector can use some shared discovery code, this is a bit nicer than having them both completely written, but I feel there could be some updates here to make less verbose in each
The common module needs to be setup with Detekt
Add test to ensure the correct issues are present
The test application needs to be updated for the latest checks
It looks like the lint checks won't work in the test module, they won't work if they are set to compileOnly either. To get around this, make the common module a sourceSet in each of the other modules.
Features
Documentation
Code Refactoring
Bug Fixes
TestsChore
Contributors |
Chesire
commented
May 8, 2020
lintrules-common/src/main/java/com/chesire/lintrules/common/issues/MultipleSpaceIssue.kt
Outdated
Show resolved
Hide resolved
|
There should be some way to share the whole detector and issue, instead of having two implementations |
|
Maybe if the |
Change from the Severity of Information to use a Severity of Warning
A lot of the code was still duplicated between the two modules, move more of the shared logic to being defined in the BaseMultipleSpaceDetector, with the platform specific ones only needing to provide the report method. This will still work correctly as each platform has to provide its own Issue with its own Implementation, so they will still be separated
|
More of the logic is now within the shared directory |
The consumers shouldn't need to know the increase the offsets when reporting, do it before sending to the report method
|
README also needs to be updated |
Update readme to mention the new lint rules
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add a Gradle and XML rule that will notify if a double space is detected. This required a little refactor to add some form of shared code, so now the modules have a common module added as a sourceSet.
Trying to add the module itself as a dependency to the main modules seemed to have caused issues, so a sourceset seems to be the way to make it work.