QDoc: Assume that aggregates have a single meaningful include file

QDoc's `Aggregates`, which are used to internally represent "containing"
elements, such as cpp classes, have a concept of "include files".

"include files" represent the name of a file or similar construct from
which the aggregate can be obtained the sources.
For example, the name an header file declaring a cpp class.

Aggregates, up to now, exposed an interface that allowed to set
multiple "include file" for each aggregate.

Furthermore, QDoc allows the user to manually specify include files
through the use of the "\inheaderfile" command.

Those include files were later used by QDoc to show information about
the aggregate, In particular, the generator would show an "include
requisite" to help a user understand where to import the aggregate from.

The multiplicity of the include files had effects on later parts of the
code that either set or read them. For example, the
generators whose code required handling the multiplicity to show the
required information on multiple lines.

In practice, this never took effect as, at least in Qt's codebase, no
aggregate has more than 1 include file.

While it is not unthinkable of wanting to have more than one include
file, we consider improbable for it to happen.

In general, include file will mostly be used for classes, which we
expect to be able to be meaningfully included from a specific file only.

As such, the interface for `Aggregates` is now modified to expose a
single include file per aggregate.

The previous methods were removed and a singular version of them was
introduced.

The internal state of `Aggregate` was modified to take this into account
storing a single optional include file instead of a vector of them.

A singular include file was set during the construction of `HeaderNode`.
The constructor is now modified to use the new interface.

Include files are then generally set only during the code parsing phase.
In particular, each file-name coming from an "\inheaderfile" command was
appended to the vector of include files. Furthermore, each aggregate
either inherited the include files of one of its parent or generated one
from the aggregate's name when no sensible parent was available, unless
a previous "\inheaderfile" command for the aggregate was encountered.

The code is now changed to take into account the lack of multiplicity.
The semantic is mostly preserved from the previous version and, in the
case of the Qt's codebase, is preserved entirely due to no aggregate
with more than one include file ever being produced.

Some slight semantic breakages are, albeit, included with this change.

Previously, if the user provided more than one "\inheaderfile" command,
the arguments from all of them would be preserved and saved in the
relevant aggregate.

With the current change, each "\inheaderfile" after the first will
overwrite the preceding value, such that only the last encountered
command will be taken, effectively, into account.

This breakage would currently be opaque for the user. It is later
advised to better expose this behavior through changes to `DocParser`,
albeit those changes are not expected to be performed right away as a
more thorough inspection of `DocParser` must be made in general.

Furthermore, the previous version would set include files that could be
the empty string, specifically as they came from an "\inheaderfile"
command.
The new version ignore empty include files, such that it is ensured
that, if an include file was ever provided, that include file is not an
empty string.

This breakage improves on the previous version, where later user of the
include files had to perform possibly costly filtering operations to
avoid empty strings, who are expected to have no meaning as an include
file.

To better support this change and partially help in exposing this
semantic, the new interface for the include file exposes an
std::optional and informally guarantees, but does not enforce, that if a
value is ever set it is not the empty string.

The changes to the code were made so as to ensure that this will be the
case as for the current codebase.

A more thorough enforcement was avoided as it would require deeper
changes that are currently unfeasible.

User of the include file, namely the generators, were modified to take
into account the lack of multiplicity.

The generators previously depended on `CodeMarker::markedUpIncludes` to
flatten the vector of include files into a representable string.

`CodeMarker::markedUpIncludes` is now supplanted with
`CodeMarker::markedUpInclude`, its singular version, to take into
account the lack of multiplicity.

The semantics of the method is preserved and is expected to be
equivalent to a call to `CodeMarker::markedUpIncludes` for vectors of
include files with a single element, with the exclusion of a trailing
newline.

Small clean up changes were made to
`HtmlGenerator::addIncludeFilesToMap`, which is the main consumer of
include file for the HTML format.

The method was renamed to its singular version.
Some now unnecessary operations, such as the filtering of include files
that were the empty string, are now removed.

The `text` parameter of the method, previously a `Text*`, is now modified
to a reference.
The `Text` that is passed to the method is always created in its
immediate scope and is never a nullptr, allowing us to remove and
unnecessary check.

Indeed, calling the method with a nullptr would generally be a no-op,
such that there would be no meaning to it in the first place (and there
was no flow-requirement for it to be a no-op).

A slight change to the laying out of the table in
`HtmlGenerator::generateTheTable` was included to better divide the
lines of the table.

The regression files for `tst_generatedOutput` were regenerated to take
into account the additional newlines in the tables.

Change-Id: Idebca8a74444328cd21c83be9e3f3b1706907f84
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>

1 files changed, 33 insertions, 6 deletions

diff --git a/src/qdoc/cppcodeparser.cpp b/src/qdoc/cppcodeparser.cpp
index 26d6b0f9f..6c1959503 100644
--- a/src/qdoc/cppcodeparser.cpp
+++ b/src/qdoc/cppcodeparser.cpp
@@ -479,8 +479,21 @@ void CppCodeParser::processMetaCommand(const Doc &doc, const QString &command,
 {
     QString arg = argPair.first;
     if (command == COMMAND_INHEADERFILE) {
-        if (node->isAggregate())
-            static_cast<Aggregate *>(node)->addIncludeFile(arg);
+        // TODO: [incorrect-constructs][header-arg]
+        // The emptiness check for arg is required as,
+        // currently, DocParser fancies passing (without any warning)
+        // incorrect constructs doen the chain, such as an
+        // "\inheaderfile" command with no argument.
+        //
+        // As it is the case here, we require further sanity checks to
+        // preserve some of the semantic for the later phases.
+        // This generally has a ripple effect on the whole codebase,
+        // making it more complex and increasesing the surface of bugs.
+        //
+        // The following emptiness check should be removed as soon as
+        // DocParser is enhanced with correct semantics.
+        if (node->isAggregate() && !arg.isEmpty())
+            static_cast<Aggregate *>(node)->setIncludeFile(arg);
         else
             doc.location().warning(QStringLiteral("Ignored '\\%1'").arg(COMMAND_INHEADERFILE));
     } else if (command == COMMAND_OVERLOAD) {
@@ -962,14 +975,28 @@ void CppCodeParser::processMetaCommands(NodeList &nodes, DocList &docs)
             checkModuleInclusion(node);
             if (node->isAggregate()) {
                 auto *aggregate = static_cast<Aggregate *>(node);
-                if (aggregate->includeFiles().isEmpty()) {
+
+                if (!aggregate->includeFile()) {
                     Aggregate *parent = aggregate;
                     while (parent->physicalModuleName().isEmpty() && (parent->parent() != nullptr))
                         parent = parent->parent();
+
                     if (parent == aggregate)
-                        aggregate->addIncludeFile(aggregate->name());
-                    else
-                        aggregate->setIncludeFiles(parent->includeFiles());
+                        // TODO: Understand if the name can be empty.
+                        // In theory it should not be possible as
+                        // there would be no aggregate to refer to
+                        // such that this code is never reached.
+                        //
+                        // If the name can be empty, this would
+                        // endanger users of the include file down the
+                        // line, forcing them to ensure that, further
+                        // to there being an actual include file, that
+                        // include file is not an empty string, such
+                        // that we would require a different way to
+                        // generate the include file here.
+                        aggregate->setIncludeFile(aggregate->name());
+                    else if (aggregate->includeFile())
+                        aggregate->setIncludeFile(*parent->includeFile());
                 }
             }
         }