apache / druid

Apache Druid: a high performance real-time analytics database.
https://druid.apache.org/
Apache License 2.0
13.45k stars 3.7k forks source link

Statically prohibit missing format parameters in Druid's exception classes #8333

Open leventov opened 5 years ago

leventov commented 5 years ago

https://youtrack.jetbrains.com/issue/IDEA-175327 doesn't allow to prevent bugs like fixed in #8331 using IntelliJ's "Malformed format string" inspection. However, there is a simple workaround: make constructors in all Druid's exception classes like RE, ISE, IAE, etc. private, expose static factory methods (e. g. RE.of()), and put them into the MalformedFormatString inspection config.

This would be a follow-up of #4474.

asdf2014 commented 5 years ago

@leventov This is a good idea :+1: Alternatively, we can also install the LGTM robot directly in the project. It automatically performs code reviews for each Pull Request to prevent unintentional bringing these issues to the project. For example, the asdf2014/algorithm project.

Pull Request: image Shields: image

asdf2014 commented 5 years ago

@leventov BTW, we need an admin or the owner of Druid to install it, at least I don't have this permission :sweat_smile:

https://lgtm.com/projects/g/apache/incubator-druid/ci/

image

leventov commented 5 years ago

@asdf2014 this is managed on the ApacheInfra level and could be prohibited because they have a white list of integrations. See https://issues.apache.org/jira/browse/INFRA-15096.

asdf2014 commented 5 years ago

@leventov I see. Thanks a lot.

ccaominh commented 5 years ago

At least one apache project seems to have enabled lgtm github integration: https://issues.apache.org/jira/browse/INFRA-17226

lgtm github integration in action on apache geode:

leventov commented 5 years ago

@ccaominh thanks for note, I've opened https://issues.apache.org/jira/browse/INFRA-18899