diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a628b7e57..a31af70c02 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -440,7 +440,7 @@ if (MRDOCS_BUILD_TESTS) DEPENDS mrdocs.rnc) add_custom_target(mrdocs_rng ALL DEPENDS mrdocs.rng) - file(GLOB_RECURSE XML_SOURCES CONFIGURE_DEPENDS test-files/*.xml) + file(GLOB_RECURSE XML_SOURCES CONFIGURE_DEPENDS test-files/golden-tests/*.xml) add_test(NAME xml-lint COMMAND ${LIBXML2_XMLLINT_EXECUTABLE} --dropdtd --noout --relaxng ${CMAKE_CURRENT_BINARY_DIR}/mrdocs.rng ${XML_SOURCES} diff --git a/docs/modules/ROOT/pages/generators.adoc b/docs/modules/ROOT/pages/generators.adoc index 94f7110592..7d057bc98a 100644 --- a/docs/modules/ROOT/pages/generators.adoc +++ b/docs/modules/ROOT/pages/generators.adoc @@ -862,25 +862,25 @@ The location object has the following properties: |=== |Property |Type| Description -| `path` +| `fullPath` | `string` -| The path of the source file. +| The full path of the source file. -| `file` +| `shortPath` | `string` -| The filename of the source file. +| The path of the source file relative to the search directories. + +| `sourcePath` +| `string` +| The path of the source file relative to the `source-root`. | `line` | `integer` -| The line number of the symbol. - -| `kind` -| `string` -| The kind of file (e.g., `source`, `system`, `other`). +| The line number of the symbol at this location. | `documented` | `bool` -| Whether the symbol is documented. +| Whether the symbol is documented at this location. |=== [#tparam-fields] diff --git a/docs/mrdocs.schema.json b/docs/mrdocs.schema.json index 281d139942..17ff4f5e8d 100644 --- a/docs/mrdocs.schema.json +++ b/docs/mrdocs.schema.json @@ -225,7 +225,7 @@ }, "source-root": { "default": "", - "description": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths.", + "description": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths. This should typically be the root directory of the git project, as relative paths formed from it can be used to create links to these source files in the repository. Templates use the `base-url` option to create links to the source code.", "title": "Path to the root directory of the source code", "type": "string" }, diff --git a/include/mrdocs/Metadata/Source.hpp b/include/mrdocs/Metadata/Source.hpp index 2adeee27f0..b331958e1e 100644 --- a/include/mrdocs/Metadata/Source.hpp +++ b/include/mrdocs/Metadata/Source.hpp @@ -51,20 +51,20 @@ struct MRDOCS_DECL { /** The full file path */ - std::string Path; + std::string FullPath; - /** Name of the file + /** The file path relative to one of the search directories */ - std::string Filename; + std::string ShortPath; + + /** The file path relative to the source-root directory + */ + std::string SourcePath; /** Line number within the file */ unsigned LineNumber = 0; - /** The kind of file this is - */ - FileKind Kind = FileKind::Source; - /** Whether this location has documentation. */ bool Documented = false; @@ -72,15 +72,15 @@ struct MRDOCS_DECL //-------------------------------------------- Location( - std::string_view const filepath = {}, - std::string_view const filename = {}, + std::string_view const full_path = {}, + std::string_view const short_path = {}, + std::string_view const source_path = {}, unsigned const line = 0, - FileKind const kind = FileKind::Source, bool const documented = false) - : Path(filepath) - , Filename(filename) + : FullPath(full_path) + , ShortPath(short_path) + , SourcePath(source_path) , LineNumber(line) - , Kind(kind) , Documented(documented) { } @@ -98,7 +98,7 @@ struct LocationEmptyPredicate constexpr bool operator()( Location const& loc) const noexcept { - return loc.Filename.empty(); + return loc.ShortPath.empty(); } }; diff --git a/include/mrdocs/Support/Path.hpp b/include/mrdocs/Support/Path.hpp index 878d9b6171..a4cf2447c7 100644 --- a/include/mrdocs/Support/Path.hpp +++ b/include/mrdocs/Support/Path.hpp @@ -241,6 +241,12 @@ std::string makePosixStyle( std::string_view pathName); +/** Check if the path is posix style. +*/ +MRDOCS_DECL +bool +isPosixStyle(std::string_view pathName); + /** Return the filename with a new or different extension. @param fileName The absolute or relative path diff --git a/mrdocs.rnc b/mrdocs.rnc index baa8e5974e..bf0aee0287 100644 --- a/mrdocs.rnc +++ b/mrdocs.rnc @@ -326,7 +326,9 @@ grammar Location = element file { - attribute path {text}, + # attribute full-path {text}, + attribute short-path {text}, + attribute source-path {text}, attribute line {text}, attribute class {"def"} ?, empty diff --git a/share/mrdocs/addons/generator/common/partials/location/source.hbs b/share/mrdocs/addons/generator/common/partials/location/source.hbs index dc7cf379fa..094c4fbadf 100644 --- a/share/mrdocs/addons/generator/common/partials/location/source.hbs +++ b/share/mrdocs/addons/generator/common/partials/location/source.hbs @@ -11,9 +11,9 @@ --}} Declared in {{#>markup/code~}} - {{ str "<" }}{{#unless (and @root.config.base-url (eq dcl.kind "source"))~}} - {{dcl.file}} + {{ str "<" }}{{#if (and @root.config.base-url dcl.shortPath dcl.sourcePath)~}} + {{#>markup/a href=(concat @root.config.base-url dcl.sourcePath '#L' dcl.line)}}{{dcl.shortPath}}{{/markup/a}} {{~else~}} - {{#>markup/a href=(concat @root.config.base-url dcl.file '#L' dcl.line)}}{{dcl.file}}{{/markup/a}} - {{~/unless~}}{{ str ">" }} + {{dcl.shortPath}} + {{~/if~}}{{ str ">" }} {{~/markup/code}} diff --git a/src/lib/AST/ASTVisitor.cpp b/src/lib/AST/ASTVisitor.cpp index 6fafebaa41..1ab9e0f5a5 100644 --- a/src/lib/AST/ASTVisitor.cpp +++ b/src/lib/AST/ASTVisitor.cpp @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -43,78 +44,6 @@ namespace clang::mrdocs { -auto -ASTVisitor::FileInfo::build( - std::span const> /* search_dirs */, - std::string_view const file_path, - std::string_view const sourceRoot) -> ASTVisitor::FileInfo { - FileInfo file_info; - file_info.full_path = std::string(file_path); - file_info.short_path = file_info.full_path; - - std::ptrdiff_t best_length = 0; - auto check_dir = [&]( - std::string_view const dir_path, - FileKind const kind) - { - auto NI = llvm::sys::path::begin(file_path); - auto const NE = llvm::sys::path::end(file_path); - auto DI = llvm::sys::path::begin(dir_path); - auto const DE = llvm::sys::path::end(dir_path); - - for(; NI != NE; ++NI, ++DI) - { - // reached the end of the directory path - if(DI == DE) - { - // update the best prefix length - if(std::ptrdiff_t length = - NI - llvm::sys::path::begin(file_path); - length > best_length) - { - best_length = length; - file_info.kind = kind; - return true; - } - break; - } - // separators always match - if(NI->size() == 1 && DI->size() == 1 && - llvm::sys::path::is_separator(NI->front()) && - llvm::sys::path::is_separator(DI->front())) - continue; - // components don't match - if(*NI != *DI) - break; - } - return false; - }; - - bool const in_sourceRoot = check_dir( - sourceRoot, FileKind::Source); - - // only use a sourceRoot relative path if we - // don't find anything in the include search directories - // bool any_match = false; - // for (auto const& [dir_path, kind]: search_dirs) - // { - // any_match |= check_dir(dir_path, kind); - // } - - // KRYSTIAN TODO: if we don't find any matches, - // make the path relative to sourceRoot and return it - - // override the file kind if - // the file was found in sourceRoot - if (in_sourceRoot) - { - file_info.kind = FileKind::Source; - } - - file_info.short_path.erase(0, best_length); - return file_info; -} - ASTVisitor:: ASTVisitor( const ConfigImpl& config, @@ -139,68 +68,6 @@ ASTVisitor( // used somewhere MRDOCS_ASSERT(context_.getTraversalScope() == std::vector{context_.getTranslationUnitDecl()}); - - auto make_posix_absolute = [&](std::string_view const old_path) - { - llvm::SmallString<128> new_path(old_path); - if(! llvm::sys::path::is_absolute(new_path)) - { - // KRYSTIAN FIXME: use FileManager::makeAbsolutePath? - auto const& cwd = source_.getFileManager(). - getFileSystemOpts().WorkingDir; - // we can't normalize a relative path - // without a base directory - // MRDOCS_ASSERT(! cwd.empty()); - llvm::sys::fs::make_absolute(cwd, new_path); - } - // remove ./ and ../ - llvm::sys::path::remove_dots(new_path, true, llvm::sys::path::Style::posix); - // convert to posix style - llvm::sys::path::native(new_path, llvm::sys::path::Style::posix); - return std::string(new_path); - }; - - Preprocessor& PP = sema_.getPreprocessor(); - HeaderSearch& HS = PP.getHeaderSearchInfo(); - std::vector> search_dirs; - search_dirs.reserve(HS.search_dir_size()); - // first, convert all the include search directories into POSIX style - for (const DirectoryLookup& DL : HS.search_dir_range()) - { - OptionalDirectoryEntryRef DR = DL.getDirRef(); - // only consider normal directories - if(! DL.isNormalDir() || ! DR) - continue; - // store the normalized path - search_dirs.emplace_back( - make_posix_absolute(DR->getName()), - DL.isSystemHeaderDirectory() ? - FileKind::System : FileKind::Other); - } - - std::string const sourceRoot = make_posix_absolute(config_->sourceRoot); - auto cacheFileInfo = [&](const FileEntry* entry) - { - // "try" implies this may fail, so fallback to getName - // if an empty string is returned - std::string_view file_path = - entry->tryGetRealPathName(); - files_.emplace( - entry, - FileInfo::build( - search_dirs, - make_posix_absolute(file_path), - sourceRoot)); - }; - - // build the file info for the main file - cacheFileInfo(source_.getFileEntryForID(source_.getMainFileID())); - - // build the file info for all included files - for(const FileEntry* file : PP.getIncludedFiles()) - { - cacheFileInfo(file); - } } void @@ -829,9 +696,7 @@ populate( { return; } - I.DefLoc.emplace(file->full_path, - file->short_path, line, file->kind, - documented); + I.DefLoc.emplace(file->full_path, file->short_path, file->source_path, line, documented); } else { @@ -840,15 +705,13 @@ populate( [line, file](const Location& l) { return l.LineNumber == line && - l.Path == file->full_path; + l.FullPath == file->full_path; }); if (existing != I.Loc.end()) { return; } - I.Loc.emplace_back(file->full_path, - file->short_path, line, file->kind, - documented); + I.Loc.emplace_back(file->full_path, file->short_path, file->source_path, line, documented); } } @@ -2560,8 +2423,8 @@ shouldExtract( // This is not a scoped promotion because // parents and members should also assume // the same base extraction mode. - if (checkFileFilters(D) && - checkSymbolFilters(D)) + if (checkSymbolFilters(D) && + checkFileFilters(D)) { mode_ = ExtractionMode::Regular; } @@ -2569,17 +2432,17 @@ shouldExtract( return true; } + // Check if this symbol should be extracted according + // to its qualified name. This checks if it matches + // the symbol patterns and if it's not excluded. + MRDOCS_CHECK_OR(checkSymbolFilters(D), false); + // Check if this symbol should be extracted according // to its location. This checks if it's in one of the // input directories, if it matches the file patterns, // and it's not in an excluded file. MRDOCS_CHECK_OR(checkFileFilters(D), false); - // Check if this symbol should be extracted according - // to its qualified name. This checks if it matches - // the symbol patterns and if it's not excluded. - MRDOCS_CHECK_OR(checkSymbolFilters(D), false); - return true; } @@ -2822,36 +2685,138 @@ ASTVisitor::FileInfo* ASTVisitor:: findFileInfo(clang::SourceLocation const loc) { - if (loc.isInvalid()) - { - return nullptr; - } - // KRYSTIAN FIXME: we really should not be - // calling getPresumedLoc this often, + MRDOCS_CHECK_OR(!loc.isInvalid(), nullptr); + + // KRYSTIAN FIXME: we really should not be calling getPresumedLoc this often, // it's quite expensive auto const presumed = source_.getPresumedLoc(loc, false); - if (presumed.isInvalid()) + MRDOCS_CHECK_OR(!presumed.isInvalid(), nullptr); + + const FileEntry* entry = source_.getFileEntryForID( presumed.getFileID()); + MRDOCS_CHECK_OR(entry, nullptr); + + // Find in the cache + if (auto const it = files_.find(entry); it != files_.end()) { - return nullptr; + return std::addressof(it->second); } - const FileEntry* file = - source_.getFileEntryForID( - presumed.getFileID()); - // KRYSTIAN NOTE: i have no idea under what - // circumstances the file entry would be null - if (!file) + + // Build FileInfo + auto const FI = buildFileInfo(entry); + MRDOCS_CHECK_OR(FI, nullptr); + auto [it, inserted] = files_.try_emplace(entry, std::move(*FI)); + return std::addressof(it->second); +} + +std::optional +ASTVisitor:: +buildFileInfo(FileEntry const* entry) +{ + std::string_view const file_path = entry->tryGetRealPathName(); + MRDOCS_CHECK_OR(!file_path.empty(), std::nullopt); + return buildFileInfo(file_path); +} + +ASTVisitor::FileInfo +ASTVisitor:: +buildFileInfo(std::string_view const file_path) +{ + FileInfo file_info; + file_info.full_path = file_path; + if (!files::isPosixStyle(file_info.full_path)) { - return nullptr; + file_info.full_path = files::makePosixStyle(file_info.full_path); } - // KRYSTIAN NOTE: i have no idea under what - // circumstances the file would not be in either - // the main file, or an included file - auto const it = files_.find(file); - if (it == files_.end()) + + // Attempts to get a relative path for the prefix + auto tryGetRelativePosixPath = [&file_info](std::string_view const prefix) + -> std::optional { - return nullptr; + if (files::startsWith(file_info.full_path, prefix)) + { + std::string_view res = file_info.full_path; + res.remove_prefix(prefix.size()); + if (res.starts_with('/')) + { + res.remove_prefix(1); + } + return res; + } + return std::nullopt; + }; + + auto tryGetRelativePath = [&tryGetRelativePosixPath](std::string_view const prefix) + -> std::optional + { + if (!files::isAbsolute(prefix)) + { + return std::nullopt; + } + if (files::isPosixStyle(prefix)) + { + // If already posix, we use the string view directly + // to avoid creating a new string for the check + return tryGetRelativePosixPath(prefix); + } + std::string const posixPrefix = files::makePosixStyle(prefix); + return tryGetRelativePosixPath(posixPrefix); + }; + + // Populate file relative to source-root + if (files::isAbsolute(config_->sourceRoot)) + { + if (auto shortPath = tryGetRelativePath(config_->sourceRoot)) + { + file_info.source_path = std::string(*shortPath); + } + } + + // Find the best match for the file path in the search directories + for (HeaderSearch& HS = sema_.getPreprocessor().getHeaderSearchInfo(); + DirectoryLookup const& DL : HS.search_dir_range()) + { + OptionalDirectoryEntryRef DR = DL.getDirRef(); + if (!DL.isNormalDir() || !DR) + { + // Only consider normal directories + continue; + } + StringRef searchDir = DR->getName(); + if (auto shortPath = tryGetRelativePath(searchDir)) + { + file_info.short_path = std::string(*shortPath); + return file_info; + } + } + + // Fallback to the source root + if (!file_info.source_path.empty()) + { + file_info.short_path = file_info.source_path; + return file_info; + } + + // Fallback to system search paths in PATH + std::optional const optEnvPathsStr = llvm::sys::Process::GetEnv("PATH"); + MRDOCS_CHECK_OR(optEnvPathsStr, file_info); + std::string const& envPathsStr = *optEnvPathsStr; + for (auto const envPaths = llvm::split(envPathsStr, llvm::sys::EnvPathSeparator); + auto envPath: envPaths) + { + if (!files::isAbsolute(envPath)) + { + continue; + } + if (auto shortPath = tryGetRelativePath(envPath)) + { + file_info.short_path = std::string(*shortPath); + return file_info; + } } - return &it->second; + + // Fallback to the full path + file_info.short_path = file_info.full_path; + return file_info; } template InfoTy> @@ -2886,9 +2851,10 @@ ASTVisitor:: upsert(DeclType* D) { AccessSpecifier access = getAccess(D); - MRDOCS_CHECK_MSG( - shouldExtract(D, access), - "Symbol should not be extracted"); + if (!shouldExtract(D, access)) + { + return Unexpected(Error("Symbol should not be extracted")); + } SymbolID const id = generateID(D); MRDOCS_CHECK_MSG(id, "Failed to extract symbol ID"); diff --git a/src/lib/AST/ASTVisitor.hpp b/src/lib/AST/ASTVisitor.hpp index 18da626a67..fec2b9aee3 100644 --- a/src/lib/AST/ASTVisitor.hpp +++ b/src/lib/AST/ASTVisitor.hpp @@ -89,21 +89,14 @@ class ASTVisitor */ struct FileInfo { - static - FileInfo - build( - std::span> search_dirs, - std::string_view file_path, - std::string_view sourceRoot); - // The full path of the file. std::string full_path; // The file path relative to a search directory. std::string short_path; - // The kind of the file. - FileKind kind = FileKind::Source; + // The file path relative to the source-root directory. + std::string source_path; // Whether this file passes the file filters std::optional passesFilters; @@ -902,13 +895,9 @@ class ASTVisitor This function will return a pointer to a FileInfo object for a given Clang FileEntry object. - If the FileInfo object does not exist, the function - will construct a new FileInfo object and add it to - the files_ map. - - The map of files is created during the object - construction, and is populated with the FileInfo - for each file in the translation unit. + If the FileInfo object does not exist in the cache, + the function will build a new FileInfo object and + add it to the files_ map. @param file The Clang FileEntry object to get the FileInfo for. @@ -917,6 +906,49 @@ class ASTVisitor FileInfo* findFileInfo(clang::SourceLocation loc); + /* Build a FileInfo for a FileEntry + + This function will build a FileInfo object for a + given Clang FileEntry object. + + The function will extract the full path and short + path of the file, and store the information in the + FileInfo object. + + This function is used as an auxiliary function to + `findFileInfo` when the FileInfo object does not + exist in the cache. + + @param file The Clang FileEntry object to build the FileInfo for. + @return the FileInfo object. + + */ + std::optional + buildFileInfo(FileEntry const* entry); + + /* Build a FileInfo for a string path + + This function will build a FileInfo object for a + given file path. + + The function will extract the full path and short + path of the file, and store the information in the + FileInfo object. + + The search paths will be used to identify the + short path of the file relative to the search + directories. + + This function is used as an auxiliary function to + `buildFileInfo` once the file path has been extracted + from the FileEntry object. + + @param file_path The file path to build the FileInfo for. + @return the FileInfo object. + */ + FileInfo + buildFileInfo(std::string_view file_path); + /* Result of an upsert operation This struct is used to return the result of an diff --git a/src/lib/Gen/xml/XMLWriter.cpp b/src/lib/Gen/xml/XMLWriter.cpp index fa6f2c068c..a950dba8de 100644 --- a/src/lib/Gen/xml/XMLWriter.cpp +++ b/src/lib/Gen/xml/XMLWriter.cpp @@ -626,10 +626,14 @@ XMLWriter:: writeSourceInfo( SourceInfo const& I) { - if(I.DefLoc) + if (I.DefLoc) + { writeLocation(*I.DefLoc, true); - for(auto const& loc : I.Loc) + } + for (auto const& loc: I.Loc) + { writeLocation(loc, false); + } } void @@ -639,9 +643,11 @@ writeLocation( bool def) { tags_.write("file", {}, { - { "path", loc.Filename }, + // { "full-path", loc.FullPath }, + { "short-path", loc.ShortPath }, + { "source-path", loc.SourcePath }, { "line", std::to_string(loc.LineNumber) }, - { "class", "def", def } }); + { "class", "def", def }}); } //------------------------------------------------ diff --git a/src/lib/Lib/ConfigOptions.json b/src/lib/Lib/ConfigOptions.json index dee5e87e5b..6e76d703b8 100644 --- a/src/lib/Lib/ConfigOptions.json +++ b/src/lib/Lib/ConfigOptions.json @@ -42,7 +42,7 @@ { "name": "source-root", "brief": "Path to the root directory of the source code", - "details": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths.", + "details": "Path to the root directory of the source code. This path is used as a default for input files and a base for relative paths formed from absolute paths. This should typically be the root directory of the git project, as relative paths formed from it can be used to create links to these source files in the repository. Templates use the `base-url` option to create links to the source code.", "type": "dir-path", "default": "", "relative-to": "", diff --git a/src/lib/Metadata/Reduce.cpp b/src/lib/Metadata/Reduce.cpp index 8f58d6af16..877a413dce 100644 --- a/src/lib/Metadata/Reduce.cpp +++ b/src/lib/Metadata/Reduce.cpp @@ -27,8 +27,8 @@ struct LocationEqual Location const& L1) const noexcept { return - std::tie(L0.LineNumber, L0.Filename) == - std::tie(L1.LineNumber, L1.Filename); + std::tie(L0.LineNumber, L0.FullPath) == + std::tie(L1.LineNumber, L1.FullPath); } }; @@ -43,8 +43,8 @@ struct LocationLess Location const& L1) const noexcept { return - std::tie(L0.LineNumber, L0.Filename) < - std::tie(L1.LineNumber, L1.Filename); + std::tie(L0.LineNumber, L0.FullPath) < + std::tie(L1.LineNumber, L1.FullPath); } }; diff --git a/src/lib/Metadata/Source.cpp b/src/lib/Metadata/Source.cpp index 1b77dccfa8..f3878036f3 100644 --- a/src/lib/Metadata/Source.cpp +++ b/src/lib/Metadata/Source.cpp @@ -39,10 +39,10 @@ tag_invoke( IO& io, Location const& loc) { - io.map("path", loc.Path); - io.map("file", loc.Filename); + io.map("fullPath", loc.FullPath); + io.map("shortPath", loc.ShortPath); + io.map("sourcePath", loc.SourcePath); io.map("line", loc.LineNumber); - io.map("kind", loc.Kind); io.map("documented", loc.Documented); } diff --git a/src/lib/Support/Path.cpp b/src/lib/Support/Path.cpp index 0dd40b2c96..5c240f6452 100644 --- a/src/lib/Support/Path.cpp +++ b/src/lib/Support/Path.cpp @@ -263,16 +263,31 @@ makeAbsolute( } std::string -makePosixStyle( - std::string_view pathName) +makePosixStyle(std::string_view pathName) { - std::string result(pathName); - for(auto& c : result) - if(c == '\\') - c = '/'; - return result; + SmallPathString result(pathName); + llvm::sys::path::native(result, llvm::sys::path::Style::posix); + return std::string(result); +} + +bool +isPosixStyle(std::string_view pathName) +{ + namespace path = llvm::sys::path; + + if(pathName.empty()) + { + return true; + } + llvm::StringRef separator = llvm::sys::path::get_separator(path::Style::windows); + if (pathName.find(separator) != llvm::StringRef::npos) + { + return false; + } + return true; } + std::string withExtension( std::string_view fileName, diff --git a/test-files/golden-tests/core/libcxx.xml b/test-files/golden-tests/core/libcxx.xml index fd73826747..f961338fbd 100644 --- a/test-files/golden-tests/core/libcxx.xml +++ b/test-files/golden-tests/core/libcxx.xml @@ -5,7 +5,7 @@