Running static analysis from Gradle
SquidDev opened this issue ยท 3 comments
As #235 has shown, there's definitely some still some code-style issues within CC. While we do have IntelliJ formatting/inspection configs in the root directory, this doesn't work very well for people using other IDEs (and is somewhat harder to do as part of the build process).
It would be pretty nice if we could setup decent configurations for CheckStyle and possibly SpotBugs, so we can run them from Gradle. I know the latter can be rather aggressive, so we may want to start with the settings turned down quite low and slowly ramp them up.
I'd also quite like to get some static analysis running on the Lua side, even if just to catch undeclared variables. I've been building some tooling as part of #133 which should help with this, but it may be worth trying to get LuaCheck running in the meantime.
One other thing possibly worth doing is using CheckStyle's ImportControl
to enforce some separation between CC's various components:
dan200.computercraft.api
shouldn't depend on any other CC package.dan200.computercraft.core
shouldn't use Minecraft classes (aside fromTerminal.java
, due to serialisation).- Possibly also block some external libraries from being referenced at all (
scala.*
,com.sun.*
, etc...).
It may be worth seeing if we want to split up dan200.computercraft.util
into "core" and "non-core" packages. Or alternatively move some of the Minecraft-specific things into shared
. For instance, TickScheduler
should probably be moved to make it clear that it's part of the core infrastructure, rather than just a helper class.
So I added CheckStyle and the license plugin last week, and they caught a couple of issues (not actual bugs, but definite problems).
I've had a bit of a fiddle with SpotBugs, and found a couple of possible bugs in our code - I need to play around a little more, fix those and hopefully enable it by default.
A couple of other things it may be worth trying instead:
- Error Prone. Like SpotBugs, probably more aggressive than I'd like, but still...
- Checker Framework. This is something I've wanted to do for a while. Some things I'd like to see if we can enforce with this (or with custom checkers):
- Enforce purity on
writeDescription
methods (see #245). - Some sort of tainting for server-thread only methods - can we ensure that peripheral methods are thread safe (likewise, client/server methods). This feels like a modification of the "Gui Effect checker".
- Enforce purity on