Add control flow graph implementation

[jgardn3r]

This PR adds the notion of a control flow graph. Control flow graphs are a stepping stone in data flow analysis and symbolic execution.

With just the control flow graph alone, two new rules could be implemented:

• RedundantJump
• LoopExecutingAtMostOnce

In addition to that, some more API traversal methods were added:

• RepeatStatementNode::getGuardExpression
• RepeatStatementNode::getStatements
• CaseStatementNode::getSelectorExpression
• CaseItemStatementNode::getStatement

The thinking is that the control flow graph will be internal for a while before being exposed in the stable API. In light of this, I have made a ControlFlowGraph interface in the API, but there are no public ways to create/interact with it.

[fourls]

I've gone through all the core CFG stuff now, I've got a few initial comments that I'll put out so we can get started. I have more reviewing to do still, don't worry 😄

Overall, looks very cool and seems to work well! I have a few broad thoughts/questions:

• The core CFG stuff relies heavily on public fields, which is very uncomfortably un-encapsulated in my view. I found it difficult to follow since there was very little "contract" about what could change. I see this comes directly from SonarJava - can we deviate from them and redesign for more encapsulation and better OO vibes?

• In something this logically complex I think it'll be vital to have plenty of explanatory comments if we're going to have any chance of maintenance + continual improvement. At the moment it's very sparse.

• Rules shouldn't instantiate ...Impl classes themselves, that should be done through the API layer (e.g. a ControlFlowGraphFactory that builds a new ControlFlowGraph, for example). Core rules can use Impl behaviour by casting to ControlFlowGraphImpl if they absolutely need to.

• I don't really see the need to keep the ControlFlowGraph interface empty for the time being, although if this is something that you think would be beneficial then I don't really mind. Would the API change all that much after this PR is merged?

[jgardn3r]

I didn't respond to each review comment because many things have changed and it is practically a rewrite. I have tried to:

1. Improve documentation and explanatory comments
2. Use immutability
3. Create an API that isn't public

[jgardn3r]

There are a few aspects of the current shape that I am not completely happy with yet that I think would be worth discussing/brainstorming:

• The modelling of the successors
  • getSuccessors()
    • Returns an interface that allows you to check what kind of successors there are
    • Is this worth it?
    • This replaces the sonar-java approach of having a flag for a bunch of different things, e.g., isFinallyBlock, isCatchBlock
  • getSuccessorBlocks()
    • This is the successor-type agnostic way of getting all of the successor blocks
    • Generically useful for traversing blocks in rules, etc.
  • Object reference permanence - not necessarily a bad thing, just worth noting
    • When constructing the blocks the successors are treated as immutable
    • If there is a change to the successors a new successor object is used to replace
    • Blocks reference each other using address, rather than ID
  • An idea I had is only making the strongly coupled object as the end and using IDs internally

[cirras]

There are some incomplete conversations, including completing the documentation sweep in the API layer.

Importantly, I'd like to see @returns, @params, etc. in the javadocs.
This does lead to some repetition, but that's the convention across the project.

[fourls]

Looks really great, functionality is really impressive and I couldn't find any obvious bugs. I have a lot of comments with questions, ideas for renaming/refactoring, and recs for where to put more tests.

The domain of this PR is really complex, confusing, and wide-ranging - we're basically reviewing an entirely new semantic model of Delphi code, and so there's so much that it's difficult to deep-dive on any single thing. In my view, that means that we need to reduce the cognitive noise as much as possible by improving naming/modelling and making sure testing is comprehensive (since we're relying on them to assert most of the functionality is correct).

[fourls]

Also, I'm not sure if you've noticed @jgardn3r but there are still a few unresolved conversations from @cirras 's first round of review.

[fourls]

Looks good! Fixed most of my concerns from the previous round of review.
This round I had a bit more of a look at the CFG tests to see if everything matched up - mostly good, I have a few comments.

[fourls]

Looks good! Only a few quite minor things now. Not sure why GitHub's split them up into a few mini reviews.

You'll have to rebase at some point as there are merge conflicts + the workflows have stopped running.

[cirras]

I have some pedantic comments about the javadocs and API layer.

Once these are sorted, I'm happy.

[fourls]

Looks good to me! This will be a really great new feature that can power a lot of really interesting rules.

[jgardn3r]

Went a little overzealous with the delete button and failed to compile again. All @cirras comments should be addressed now.

[cirras]

This time for sure, maybe!

[cirras]

I'm happy with this now - well done. 👍

There's a trivial follow-up, which is releasing the API for custom plugins. We'll do that once we've built some more things on this feature and are confident that we like the current shape of the CFG API.