Move FlowSummaryImpl.qll to dataflow pack

[hvitved]

This PR turns FlowSummaryImpl.qll (and AccessPathSyntax.qll) into a proper parameterized module, and moves it to the shared dataflow pack.

Previously, the exposed SummarizedCallable class would look like

abstract class SummarizedCallable extends SummarizedCallableBase {
  predicate propagatesFlow(
    SummaryComponentStack input, SummaryComponentStack output, boolean preservesValue
  ) {
    none()
  }

  predicate propagatesFlowExt(
    string input, string output, boolean preservesValue
  ) {
    none()
  }
}

but after this PR it is simply

abstract class SummarizedCallable extends SummarizedCallableBase {
  abstract predicate propagatesFlow(
    string input, string output, boolean preservesValue
  );
}

That is, we now consider SummaryComponentStack an internal implementation detail, which is not exposed to the user (the user-facing aliases have been deprecated).

Commit-by-commit review is encouraged, and for each of the <LANG>: Use FlowSummaryImpl from dataflow pack commits, I recommend viewing the FlowSummaryImpl.qll in full, as the diff is not really meaningful (previously, it was the synced copy of share library, now it contains the inputs to the shared library).

[erik-krogh]

JS 👍

[MathiasVP]

Swift 👍

[RasmusWL]

Overall LGTM, but there are 2 things I would like to get fixed for Python 🙏

[michaelnebel]

Maybe a comment that this is where the MaD rows are being used to create SummarizedCallable's (the same for neutrals as well).

[michaelnebel]

C#: Looks good to me!
Thank you for doing this @hvitved !

[RasmusWL]

Python 👍 (thanks for the updates)

[yoff]

I can see this has already been approved by Python. I can also see that I have a pending comment, but I do not remember what it is, so I will just submit the partial review now, to see if it is still relevant.

[aschackmull]

I think I might see a bug related to the MaD provenance column. Could you verify that the presence of both a manual and a generated model for the same callable yields only the manual summary? I believe this used to work, but I think it might be broken now. Alright, I no longer think there's a bug, instead I think that most of the logic in SummarizedCallableExternal can be deleted as it appears to be useless (and that's what threw me off in the first place).

[owen-mc]

@aschackmull Do we have a test for what happens when we have a manual model and a generated model?

[hvitved]

Do we have a test for what happens when we have a manual model and a generated model?

There is at least https://github.com/github/codeql/blob/main/java/ql/src/Metrics/Summaries/GeneratedVsManualCoverage.ql.

[michaelnebel]

@aschackmull Do we have a test for what happens when we have a manual model and a generated model?

I don't think there exists such a test for Java.
But one can use the summary/1 predicate for creating such a test. (cc @owen-mc)

[aschackmull]

LGTM! Thanks for tackling this!