In php there is phpmd and phpcs that are really hard to pass when having some legacy code.
However, this means that as soon as you start mucking about in the legacy code, that code need to be covered by tests and formatted to pass any style checks and linters. That is not an insignificant amount of work so plan accordingly.
Also, I find it useful to remember that even if the code is not to my liking (for whatever reason) it might be consistent. I.e. maybe it uses some coding style I am unfamiliar with and I would rather it used another it is often detrimental to start changing it to be more inline with my preferences. Consistently poor code is often easier to reason about than code that is a mix of different preferences and patterns. This also goes for when thinking about introducing new technology to an existing code base (see the lava flow antipattern).
It took us about 18 months of gradual chipping away to get our codebase to pass a linter, enabling one rule or class of rules at a time.
We're still working on test coverage. The key is to make sure all new code is covered (enforce in code review) and then very gradually backfill the rest. Add reproduction before fixing a bug. Add end to end coverage before a refactoring.
You're just adding to the "activation potential" required to make a necessary fix in legacy code. Changes in legacy code should be minimal. Nobody who has to make a fix in legacy code should be asked to upgrade its style and whatnot, that is not related to the fix.
If it has its own conventions, just maintain those.
Or else, make a commitment to upgrade the legacy code to new conventions, as a separate, dedicated task, whose goal is to do it thoroughly.
The last thing you want is a hodgepodge where code that has been recently touched conforms to one set of conventions, and code that has not been touched conforms to another.
I think a good way to approach legacy code (at a high level, big picture) is to answer the question "how much of the value of what we are doing here is propped up by this legacy code?" If the fact that the legacy code works is paying your salary, respect that; maybe those people weren't idiots.
It teaches several ways of dealing with large untested codes in edible chunks.
One of the most memorable part is that the author defines legacy code as "code without tests".
And you can handle it by the following:
0. poke around in the code till you find a part you think you can test something in isolation. If you need some _trivial_ changes to reach that, thats okay. 1. create some kind of test that will catch you if you break something 2. refactor and understand
The book is about offering several tactics for dealing with points 0. and 1. and is very digestible, good read.
Interestingly, Feathers defines Legacy Code as code without automated tests.
the rough process goes like this:
0. you need to make a change to some code that you dont understand and that has no automated tests and no reliable documentation of what its behaviour is
1. you need to refactor the code or change some logic in order to fix the required bug / add the new feature, but you cannot do this safely without having a way to measure the impact upon the existing code's unknown behaviour.
2. a controlled way of making such changes is to first wrap the code in automated regression / characterisation tests. you need to get these automated tests in place without changing the behaviour of the code. this may be challenging. there may be a bit of a chicken and egg problem where you dont want to refactor without having regression tests, yet you need to refactor to wedge the tests in.
3. after you have some decent regression tests in place, you can try to focus in on isolating and understanding the region(s) that you need to modify to achieve your intended change.
4. it may be necessary to refactor the existing code to achieve this, or to add unit tests to key components. your safety net of regression tests gives you some confidence to do this, provided you are careful in how you refactor to get "seams" in place in the code structure to do fine-detail unit / characterisation tests.
Most of the book is tactics of roughly how to do this. from memory, some of the tips in the book are geared towards typed compiled languages (java, C++) -- e.g. you can use the compiler's type checker as a source of information to learn about how the code is structured (by changing things and trying to compile and seeing how many pages of errors you get...)
Feathers also has a blog which may give you some actionable ideas:
* https://michaelfeathers.silvrback.com/revealing-interfaces
* https://michaelfeathers.silvrback.com/to-kill-code
* https://michaelfeathers.silvrback.com/characterization-testi...
* https://michaelfeathers.silvrback.com/knowing-what-is-there
edit: another good rule of thumb:
http://wiki.c2.com/?RefactorLowHangingFruit
the most ideal scenario for getting legacy code under control is the following:
* the code itself is purely functional: no side effects, and no external dependencies on irritating things like databases, rpc calls to external services, random crap that gets shell-executed. the code also happens to be completely deterministic. the code is written in a language that is statically typed and has clever compilers, linters, automated test frameworks, editors that implement automated guaranteed-safe refactoring moves, etc.
in reality you are almost certainly going to have external state. figuring out how to isolate/mock/reproduce that in regression tests might be one of the first major challenges.
it would be very helpful if you have good support from "the business" for this work, in particular that they understand getting this legacy code under control may be a very slow and costly operation.
if you are particularly unlucky then no one left in the org will understand the "theory of the [legacy] code". in order to make safe and useful changes to the existing codebase you will need to gradually reverse engineer the theory of the code / re-learn what the requirements and major constraints on the system are. there's a great essay by Naur (of e.g. bNf fame) about this: http://pages.cs.wisc.edu/~remzi/Naur.pdf (previous HN discussion: https://news.ycombinator.com/item?id=10833278 )
there are also other strategies for dealing with this kind of situation.
probably trying to rewrite the entire thing is a poor idea, various people have written about the risks and failure modes of that approach.
it might be possible to identify one responsibility of the legacy system that needs to be changed, and can be rewritten as a separate service. see e.g. https://www.martinfowler.com/bliki/StranglerApplication.html ; http://cdn.pols.co.uk/papers/agile-approach-to-legacy-system...
Tests have also been very helpful. So say I'm in a situation where the legacy code is not style-compliant, let's forget the style-oriented analysis and double down on adding tests. Once you have good test coverage, start to clean up the styling and start to think about static code analysis as part of the build.
Yeah, that's probably not a bad approach. it also means it is acceptable to introduce new lint violations (this may be unavoidable) if the committer "pays the tax" / "pays a tithe" of fixing some existing violation that might be easier to clean up.
another way to implement this (which probably depends upon details of the version control and lint tools being used) is, for each change, query version control to identify what files have changed (or ideally subsections of files), and evaluate before/after lists of issues using the lint tool.
for example, for lint tools that only need to read the content of a source file (without e.g. executing or importing it or whatever), and support processing a single file at a time (instead of needing to scan the entire source tree) you can probably cobble something together reasonably easily if no existing tool exists.
for example, if you are dealing with a legacy python codebase that is version controlled using git, you could start using `pyflakes` that is able to analyse each file in isolation, and merely needs to read the content of the file and not import or execute it. it isnt that hard to query git to discover which files were changed in a commit (e.g. `git show --name-status <commit>`) or view the content of a file at an arbitrary revision (`git show <commit>:<path/to/file.py>`). with enough glue scripting you could use this to produce a custom report to show what the delta of issues (either a count of sets of the actual specific issues) is for each file touched by a commit, without having to scan the entire codebase.
My solution is to ignore the parts that can't be easily fixed. List them up in an issue or ticket somewhere to fix later if necessary. For now, modernising the majority of the codebase is a more pressing issue than nitpicking preferences.
on the other hand, sometimes in a startup-ish scenario it is optimal from a business perspective to bash out code to add features to drive revenue and worry about tech debt for the far future hypothetical scenario of the startup turning into a profitable business that can pay to bring people in to rewrite everything.
that is, optimal in the sense of the alternative being slowly writing very well thought out and engineered code and running out of money before having enough customers to justify a longer-term engineering focus
OTOH, when there is no time available for this kind of care, i just go rodeo-style, guns blazing, code crashing and long nights... fixing the most low hanging bugs, or the most critical.
See if there is tooling to do it for you. Google seems to favor this approach, seeing as they put out YAPF for python and gofmt for Go.