D-Scanner
D-Scanner copied to clipboard
dlang-community
Make KEY & MESSAGE public and required for all analyzers
I think it would make a lot of sense from an API point of view to require a key and a message for every analyzer. I recently wrote a bit of code using dscanner as API and I wanted to get a list of all supported keys. I couldn't really find one so I just hardcoded them all as strings. I think if they were abstract methods inside BaseAnalyzer it would be really easy to automate the listing of keys (KEY) and having human readable messages for the user for example for features where you can disable certain keys. (MESSAGE) Maybe message could be renamed to description too.
Also for the future if some equivalent of //dfmt off, just with attributes or similar could be implemented into dscanner, this would make it really easy to implement in BaseAnalyzer and discard issues early.
Currently some analyzers are missing MESSAGE and sometimes KEY and/or MESSAGE isn't public. This would make this a safe API to use.
Additional future benefits might include automatic generation/parsing of the config with more control for enabling/disabling analyzers per module, globally, etc.
If we change the analyzers to all only use one key, we could also drop the key argument from addErrorMessage and make sure all keys are the same as the filename to ensure consistency.
Currently some analyzers (allman for example) are doing work in the constructor or have KEY/MESSAGE as public enum mebers, so it wouldn't really add a lot to have the abstract members there. My idea was that if we had these abstract properties we could filter which ones to run before running them in a simple foreach loop, but currently this would require a bit of refactoring in some modules, making all work the same. I would actually like to have the ability to construct any analyzer without constructor arguments (so you can do it statically on an AliasSeq of defined analyzers) and a run method with generic parameters which apply to all analyzers then. To get them at compile time CTFE could probably be used with a simple wrapper taking in a T : BaseAnalyzer and just creating a new instance and returning the key/message.
Just sth to keep in mind: Some analyzers currently emit multiple keys/messages types.
Yeah those would need to be separated into multiple probably to make the resulting code the most easy to read and customize
There might be a performance penalty of traversing the tree and aggregating the same information 4x than in one pass. Also they are very often pretty related.