QDoc: Apply the FileResolver abstraction to the code

The recently added `FileResolver` abstraction encapsulate the logic that
QDoc uses for finding user-provided files in the documentation that QDoc
will need to interface with.

Previously, this process was mostly contained in `Doc::resolve_file`.

This was considered problematic as `Doc` is an extremely complex class
with many responsibilities, coupling the logic with many unrequired
preconditions and making testing of this simple logic almost
unattainable.

Furthermore, a similar logic in QDoc was provided by `Config::findFile`,
which was sometimes used instead of `Doc::resolveFile`, fracturing the
behavior of the code.

To avoid those issues, allow for the testing of the logic for file
resolution and to generally simplify the code and make it more
consistent, `Doc::resolveFile` was removed and substituted with the
`FileResolver` abstraction.

A few changes were made to support this new idiom.

- An instance of `FileResolver` is now built in `processQdocconfFile`
and passed as a reference to systems that require it during
construction.

A small exception was made for `DocParser` where the instance is passed
as a pointer through `DocParser::initialize` to support the current code
structure.

The required changes to the relevant entities constructors were made to
allow for this.

- `FileResolver` is provided as an explicit dependency to anything that
requires the capability of resolving files, such as generators.
Generally during construction.
This is intended as a first step in removing the intitialize-terminate
idiom, reducing the scope of some of the mutable state and increasing
the expliciteness of its provenance.

To allow for this, the main routing of QDoc was modified to construct
the Generators classes during the production of the documentation in
`processQdocconfFile`.

This was required as an initialized `FileResolver` is needed for the
generators to work. As `FileResolver` instead depends on a parsed
configuration, it was not possible anymore to construct the generator
once before doing anything else.

A small defensive addition to `Generator::terminate` was added, removing all
generators from the list of generators. This should not actually be
required, but it was added to ensure that the now more limited scope of
the generators and the possibility that they would be constructed more
than once would not collide with their broader, unrequired global scope.

- To take into account the simplified data requirements for
`FileResolver`, compared to the previous logic, such the usage of a
single list of directories, the extraction of relevant values from the
configuration in `processQdocconfFile` was modified to further mold the
data into a suitable format for `FileResolver`.

- As the data is relevant to searching directories is contained within
the `FileResolver` instance that underlying systems use and does not
require any processing to be retrieved, some of the caching members in,
for example, `Generator` that contained a reference to this data were
removed.

See, for example, `Generator::s_exampleDirs`.

As a consequence of all those changes, `Generator::augmentImageDirs` and
`Generator::setImageFileExtensions` were removed as unused.

- `Generator::generateExampleFilePage`, was modified from having two
overload to a single method with a default argument to reduce the bloat.

- `Generator::addImageToCopy` was modified to use a `ResolvedFile`.

The method copies an image from the examples to the output directory.
This should be done only when the image is available.

- Places where `Doc::resolveFile` were called were modified to use
`FileResolver::resolve`.

Due to this change some underlying methods were modified to require a
`ResolvedFile`.

`Doc::resolveFile` was removed as a consequence.

- `Doc::quoteFromFile`, which previously depended on `Doc::ResolveFile`,
was modified to require a `ResolvedFile`.

This lightens the responsibility of the method to quoting only, instead
of finding the file and quoting, ensuring that it is called only when
the quoting can actually happen. Furthermore, it removes the need for
some of the error reporting which is bubbled up to the caller during
file resolution.

This was further required to avoid having `Doc` require a `FileResolver`
instance, as `Doc`'s constructor is called in many places.

- The usage of a `userFriendlyFilePath` was removed from code using
`Config::findFile`.

A run of QDoc on the current documentation shows that
`userFrinedlyFilePath` is always equal to the original path that was
passed to find file except in two cases where `userFriendlyFilePath`
would be null.

Consumer code didn't seem to make any use of the null state of the
`userFriendlyFilePath` and ignoring it doesn't seem to change the output
documentation, such that it was considered safe to avoid.

It is currently unclear what the original purpose of this element was.

As `Doc::resolveFile` implicitly used `Config::findFile` and exposed
back the `userFriendlyFilePath`, this is true for all replaced usages of
the method too, which allowed ignoring the element in the first place.

- A now unused overload of `Config::findFile` taking a series of
extensions for the searched for file was removed.

- `Config::copyFile` was modified to remove some of the processing done
to its input parameters which is now unused.

A simplified version of it was retained to handle a known case that is
actually used by the code.

- `DocParser` was modified to internally make use of `FileResolver` for
when resolving files for quoting and some code handling.

It previously used a mix of `Doc::quoteFromFile` and its internal
`quoteFromFile`.

The usages were made consistent depending only on the internal
`quoteFromFile`.

`quoteFromFile` itself was further changed to take into account the
changes in `Doc::quoteFromFile`, now resolving the file itself.

- Some of the code was littered with TODO and REMARK comments to keep
track of things that were noticed during the change that will not be
addressed at this point in time.

Change-Id: Ifb2e03696f6de64dac54470f7a969d323a88c0a7
Task-number: QTBUG-100381
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>

1 files changed, 99 insertions, 73 deletions

diff --git a/src/qdoc/generator.cpp b/src/qdoc/generator.cpp
index 030dbdf06..c2beb07f6 100644
--- a/src/qdoc/generator.cpp
+++ b/src/qdoc/generator.cpp
@@ -61,14 +61,9 @@
 QT_BEGIN_NAMESPACE
 
 Generator *Generator::s_currentGenerator;
-QStringList Generator::s_exampleDirs;
-QStringList Generator::s_exampleImgExts;
 QMap<QString, QMap<QString, QString>> Generator::s_fmtLeftMaps;
 QMap<QString, QMap<QString, QString>> Generator::s_fmtRightMaps;
 QList<Generator *> Generator::s_generators;
-QStringList Generator::s_imageDirs;
-QStringList Generator::s_imageFiles;
-QMap<QString, QStringList> Generator::s_imgFileExts;
 QString Generator::s_outDir;
 QString Generator::s_outSubdir;
 QStringList Generator::s_outFileNames;
@@ -94,7 +89,8 @@ static QLatin1String quot("&quot;");
   Sets a pointer to the QDoc database singleton, which is
   available to the generator subclasses.
  */
-Generator::Generator()
+Generator::Generator(FileResolver& file_resolver)
+    : file_resolver{file_resolver}
 {
     m_qdb = QDocDatabase::qdocDB();
     s_generators.prepend(this);
@@ -949,21 +945,38 @@ void Generator::generateLinkToExample(const ExampleNode *en, CodeMarker *marker,
     generateText(text, nullptr, marker);
 }
 
-void Generator::addImageToCopy(const ExampleNode *en, const QString &file)
+void Generator::addImageToCopy(const ExampleNode *en, const ResolvedFile& resolved_file)
 {
     QDir dirInfo;
     QString userFriendlyFilePath;
-    const QString prefix("/images/used-in-examples/");
-    QString srcPath = Config::findFile(en->location(), QStringList(), s_exampleDirs, file,
-                                       s_exampleImgExts, &userFriendlyFilePath);
-    s_outFileNames << prefix.mid(1) + userFriendlyFilePath;
-    userFriendlyFilePath.truncate(userFriendlyFilePath.lastIndexOf('/'));
-    QString imgOutDir = s_outDir + prefix + userFriendlyFilePath;
+    // TODO: [uncentralized-output-directory-structure]
+    const QString prefix("/images/used-in-examples");
+
+    // TODO: Generators probably should not need to keep track of which files were generated.
+    // Understand if we really need this information and where it should
+    // belong, considering that it should be part of whichever system
+    // would actually store the file itself.
+    s_outFileNames << prefix.mid(1) + "/" + resolved_file.get_query();
+
+
+    // TODO: [uncentralized-output-directory-structure]
+    QString imgOutDir = s_outDir + prefix + "/" + QFileInfo{resolved_file.get_query()}.path();
     if (!dirInfo.mkpath(imgOutDir))
         en->location().fatal(QStringLiteral("Cannot create output directory '%1'").arg(imgOutDir));
-    Config::copyFile(en->location(), srcPath, file, imgOutDir);
+    Config::copyFile(en->location(), resolved_file.get_path(), QFileInfo{resolved_file.get_query()}.fileName(), imgOutDir);
 }
 
+// TODO: [multi-purpose-function-with-flag][generate-file-list]
+// Avoid the use of a boolean flag to dispatch to the correct
+// implementation trough branching.
+// We always have to process both images and files, such that we
+// should consider to remove the branching altogheter, performing both
+// operations in a single call.
+// Otherwise, if this turns out to be infeasible, complex or
+// possibly-confusing, consider extracting the processing code outside
+// the function and provide two higer-level dispathing functions for
+// files and images.
+
 /*!
   This function is called when the documentation for an example is
   being formatted. It outputs a list of files for the example, which
@@ -992,22 +1005,33 @@ void Generator::generateFileList(const ExampleNode *en, CodeMarker *marker, bool
     text << Atom::ParaLeft << tag << Atom::ParaRight;
     text << Atom(Atom::ListLeft, openedList.styleString());
 
-    QString path;
-    for (const auto &file : qAsConst(paths)) {
-        if (images) {
-            if (!file.isEmpty())
-                addImageToCopy(en, file);
-        } else {
-            generateExampleFilePage(en, file, marker);
+    for (const auto &path : qAsConst(paths)) {
+        auto maybe_resolved_file{file_resolver.resolve(path)};
+        if (!maybe_resolved_file) {
+            // TODO: [uncentralized-admonition][failed-resolve-file]
+            QString details = std::transform_reduce(
+                file_resolver.get_search_directories().cbegin(),
+                file_resolver.get_search_directories().cend(),
+                u"Searched directories:"_qs,
+                std::plus(),
+                [](const DirectoryPath& directory_path){ return " " + directory_path.value(); }
+            );
+
+            en->location().warning(u"(Generator)Cannot find file to quote from: %1"_qs.arg(path), details);
+
+            continue;
         }
 
+        auto file{*maybe_resolved_file};
+        if (images) addImageToCopy(en, file);
+        else        generateExampleFilePage(en, file, marker);
+
         openedList.next();
         text << Atom(Atom::ListItemNumber, openedList.numberString())
              << Atom(Atom::ListItemLeft, openedList.styleString()) << Atom::ParaLeft
-             << Atom(atomType, file) << Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK) << file
+             << Atom(atomType, file.get_query()) << Atom(Atom::FormattingLeft, ATOM_FORMATTING_LINK) << file.get_query()
              << Atom(Atom::FormattingRight, ATOM_FORMATTING_LINK) << Atom::ParaRight
              << Atom(Atom::ListItemRight, openedList.styleString());
-        path = file;
     }
     text << Atom(Atom::ListRight, openedList.styleString());
     if (!paths.isEmpty())
@@ -1589,27 +1613,6 @@ QStringList Generator::getMetadataElements(const Aggregate *inner, const QString
     return result;
 }
 
-/*!
-  Returns a relative path name for an image.
- */
-QString Generator::imageFileName(const Node *relative, const QString &fileBase)
-{
-    QString userFriendlyFilePath;
-    QString filePath = Config::findFile(relative->doc().location(), s_imageFiles, s_imageDirs,
-                                        fileBase, s_imgFileExts[format()], &userFriendlyFilePath);
-
-    if (filePath.isEmpty())
-        return QString();
-
-    QString path = Config::copyFile(relative->doc().location(), filePath, userFriendlyFilePath,
-                                    outputDir() + QLatin1String("/images"));
-    qsizetype images_slash = path.lastIndexOf("images/");
-    QString relImagePath;
-    if (images_slash != -1)
-        relImagePath = path.mid(images_slash);
-    return relImagePath;
-}
-
 QString Generator::indent(int level, const QString &markedCode)
 {
     if (level == 0)
@@ -1640,15 +1643,6 @@ void Generator::initialize()
     s_outputFormats = config.getOutputFormats();
     s_redirectDocumentationToDevNull = config.getBool(CONFIG_REDIRECTDOCUMENTATIONTODEVNULL);
 
-    s_imageFiles = config.getCanonicalPathList(CONFIG_IMAGES);
-    s_imageDirs = config.getCanonicalPathList(CONFIG_IMAGEDIRS);
-    s_exampleDirs = config.getCanonicalPathList(CONFIG_EXAMPLEDIRS);
-    s_exampleImgExts = config.getStringList(CONFIG_EXAMPLES + Config::dot + CONFIG_IMAGEEXTENSIONS);
-
-    QString imagesDotFileExtensions = CONFIG_IMAGES + Config::dot + CONFIG_FILEEXTENSIONS;
-    for (const auto &ext : config.subVars(imagesDotFileExtensions))
-        s_imgFileExts[ext] = config.getStringList(imagesDotFileExtensions + Config::dot + ext);
-
     for (auto &g : s_generators) {
         if (s_outputFormats.contains(g->format())) {
             s_currentGenerator = g;
@@ -1714,12 +1708,39 @@ void Generator::initialize()
   */
 void Generator::copyTemplateFiles(const QString &configVar, const QString &subDir)
 {
+    // TODO: [resolving-files-unlinked-to-doc]
+    // This is another case of resolving files, albeit it doesn't use Doc::resolveFile.
+    // While it may be left out of a first iteration of the file
+    // resolution logic, it should later be integrated into it.
+    // This should come naturally when the output directory logic is
+    // extracted and copying a file should require a validated
+    // intermediate format.
+    // Do note that what is done here is a bit different from the
+    // resolve file routine that is done for other user-given paths.
+    // Thas is, the paths will always be absolute and not relative as
+    // they are resolved from the configuration.
+    // Ideally, this could be solved in the configuration already,
+    // together with the other configuration resolution processes that
+    // do not abide by the same constraints that, for example, snippet
+    // resolution uses.
     Config &config = Config::instance();
     QStringList files = config.getCanonicalPathList(configVar, Config::Validate);
     if (!files.isEmpty()) {
         QDir dirInfo;
+        // TODO: [uncentralized-output-directory-structure]
+        // As with other places in the generation pass, the details of
+        // where something is saved in the output directory are spread
+        // to whichever part of the generation does the saving.
+        // It is hence complex to build a model of how an output
+        // directory looks like, as the knowledge has no specific
+        // entry point or chain-path that can be followed in full.
+        // Each of those operations should be centralized in a system
+        // that uniquely knows what the format of the output-directory
+        // is and how to perform operations on it.
+        // Later, move this operation to that centralized system.
         QString templateDir = s_outDir + QLatin1Char('/') + subDir;
         if (!dirInfo.exists(templateDir) && !dirInfo.mkdir(templateDir)) {
+            // TODO: [uncentralized-admonition]
             config.lastLocation().fatal(
                     QStringLiteral("Cannot create %1 directory '%2'").arg(subDir, templateDir));
         } else {
@@ -1789,18 +1810,6 @@ void Generator::initializeFormat()
 }
 
 /*!
-  Appends each directory path in \a moreImageDirs to the
-  list of image directories.
- */
-void Generator::augmentImageDirs(QSet<QString> &moreImageDirs)
-{
-    if (moreImageDirs.isEmpty())
-        return;
-    for (const auto &it : moreImageDirs)
-        s_imageDirs.append(it);
-}
-
-/*!
   Updates the generator's m_showInternal from the Config.
  */
 void Generator::initializeGenerator()
@@ -1950,11 +1959,6 @@ QString Generator::plainCode(const QString &markedCode)
     return t;
 }
 
-void Generator::setImageFileExtensions(const QStringList &extensions)
-{
-    s_imgFileExts[format()] = extensions;
-}
-
 int Generator::skipAtoms(const Atom *atom, Atom::AtomType type) const
 {
     int skipAhead = 0;
@@ -2037,11 +2041,33 @@ void Generator::terminate()
             generator->terminateGenerator();
     }
 
+    // REMARK: Generators currently, due to recent changes and the
+    // transitive nature of the current codebase, receive some of
+    // their dependencies in the constructor and some of them in their
+    // initialize-terminate lifetime.
+    // This means that generators need to be constructed and
+    // destructed between usages such that if multiple usages are
+    // required, the generators present in the list will have been
+    // destroyed by then such that accessing them would be an error.
+    // The current codebase calls initialize and the correspective
+    // terminate with the same scope as the lifetime of the
+    // generators.
+    // Then, clearing the list ensures that, if another generator
+    // execution is needed, the stale generators will not be removed
+    // as to be replaced by newly constructed ones.
+    // Do note that it is not clear that this will happen for any call
+    // in Qt's documentation and this should work only because of the
+    // form of the current codebase and the scoping of the
+    // initialize-terminate calls. As such, this should be considered
+    // a patchwork that may or may not be doing anything and that may
+    // break due to changes in other parts of the codebase.
+    //
+    // This is still to be considered temporary as the whole
+    // initialize-terminate idiom must be removed from the codebase.
+    s_generators.clear();
+
     s_fmtLeftMaps.clear();
     s_fmtRightMaps.clear();
-    s_imgFileExts.clear();
-    s_imageFiles.clear();
-    s_imageDirs.clear();
     s_outDir.clear();
 }