CodeQL: Separate keywords for extending and implementing abstract class

[Marcono1234]

Currently, using abstract classes in CodeQL publicly can be error-prone because extending the abstract class actually adds a new class to the union of implementing subclasses. The documentation also contains a note about this pitfall:

You must take care when you add a new subclass to an existing abstract class. Adding a subclass is not an isolated change, it also extends the abstract class since that is a union of its subclasses.

Would it therefore make sense to have two separate keywords for subclassing abstract classes? One for implementing the abstract class (e.g. implements) and one for extending / refining the union of all implementing classes (e.g. extends).

Classes implementing an abstract class need to override all abstract predicates, as usual, and contribute to the union of implementing subclasses.
Classes extending / refining an abstract class do not need to override any abstract predicates. Instead they can impose additional restrictions in their characteristics predicate on all implementing classes, or merely add additional predicates (which may call inherited abstract predicates).
Abtract subclasses of abstract classes would be considered to extend the abstract superclass, not to implement them, so they would have to use extends.

For example consider the following situation (based on #8582) where:

• JumpStmt is an abstract class
• BreakStmt and ContinueStmt implement the abstract class

Then the CodeQL code could look like this:

abstract class JumpStmt extends Stmt {
    abstract StmtParent getParent();
}

class BreakStmt implements JumpStmt extends @breakstmt {
    override StmtParent getParent() { ... }
}

class ContinueStmt implements JumpStmt extends @continuestmt {
    override StmtParent getParent() { ... }
}

A user could then define for example a subclass JumpStmtInConstructor which extends / refines the existing implementing classes without defining a new implementation. It therefore does not have to override / implement the abstract predicate:

class JumpStmtInConstructor extends JumpStmt {
    JumpStmtInConstructor() {
        getEnclosingCallable() instanceof Constructor
    }
}

However, this proposed solution would be backward incompatible with the existing usage of extends. Possibly a grace period of a few versions where extends acts like implements, but usage of extends for concrete subclasses classes is flagged as warning, could be used. And then afterwards the behavior of extends could be changed.

[alexet]

This is similar to many proposals we have had internally and we have been looking at similar features for a while.

The main issue we have been discussing was been overriding. We have been thinking about not letting you override at all in these cases as you are very likely to break the underlying implementation (as most predicates will be defined on the subclasses). However if we go down that route things get complex when you end up needing to resolve ambiguities which would normally be fixed by overriding.

There is a desire to have some way of not exposing the fact that a class is abstract and prevent overriding so we can change implementation details without it being a breaking change.

From this we have generally come to the conclusion that a stronger encapsulation that hides the implementation details of classes is more important, fully fixes this problem and is something that is desired independently. If we were to add a "refines/implements" distinction that just affected abstract class extension then it would add complexity that would be mostly be just replaced by the "encapsulation" feature.

However all of this takes time to implement and we have higher priorities at the moment as just avoiding abstract classes isn't that hard of a fix in most cases where we don't need the "extensibility".

[Marcono1234]

Thanks for the insight! Those plans definitely sound interesting. I agree that this problem does not occur that often and in most cases abstract classes can probably be avoided.