search

pgjdbc / pgjdbc

Postgresql JDBC Driver
http://jdbc.postgresql.org
BSD 2-Clause "Simplified" License
1.46k stars 828 forks source link

Consider adding LGTM integration #1295

Open vlsi opened 5 years ago

vlsi commented 5 years ago

LGTM analyses code like Checkstyle, and it seems to find interesting patters.

For instance: https://lgtm.com/projects/g/pgjdbc/pgjdbc/latest/files/pgjdbc/target/gen-src/org/postgresql/jdbc/TimestampUtils.java#L491

Note: currently our build script pre-processes sources to /target folder, and it causes LGTM to "exclude" all the sources from analysis.

I think we can configure target/gen-src as a source folder, and it should work fine then. Note: test code contains tons of "unclosed connections", so activating LGTM for test code does not seem beneficial (or we should disable those checks for test code somehow).

PS inspired by https://github.com/apache/calcite/pull/811

davecramer commented 5 years ago

For instance: https://lgtm.com/projects/g/pgjdbc/pgjdbc/latest/files/pgjdbc/target/gen-src/org/postgresql/jdbc/TimestampUtils.java#L491

I'm trying to figure out if it is really possible for useCal to be null or if the null guard is useless ?

vlsi commented 5 years ago

I'm trying to figure out if it is really possible for useCal to be null or if the null guard is useless ?

You see? The tool flags interesting code. Of course, there might be false positives, however total number of issues for the main codebase is not that high, and there are things to consider.