From a51799c279c2c8d133e800f5ae53266a2a651153 Mon Sep 17 00:00:00 2001 From: mlangkabel Date: Fri, 30 Nov 2018 16:02:10 +0100 Subject: [PATCH] logic: Store locations of errors in source_location table of DB --- src/lib/data/ErrorInfo.h | 47 ++++++- src/lib/data/indexer/TaskBuildIndex.cpp | 24 ++-- .../SharedIntermediateStorage.cpp | 6 +- .../shared_types/SharedIntermediateStorage.h | 6 +- .../shared_types/SharedStorageTypes.h | 40 ++---- src/lib/data/location/SourceLocationFile.cpp | 22 ++++ src/lib/data/location/SourceLocationFile.h | 1 + src/lib/data/parser/ParserClient.h | 3 +- src/lib/data/parser/ParserClientImpl.cpp | 13 +- src/lib/data/parser/ParserClientImpl.h | 2 +- src/lib/data/storage/IntermediateStorage.cpp | 38 ++++-- src/lib/data/storage/IntermediateStorage.h | 12 +- src/lib/data/storage/PersistentStorage.cpp | 35 +++-- src/lib/data/storage/PersistentStorage.h | 6 +- src/lib/data/storage/Storage.cpp | 5 +- src/lib/data/storage/Storage.h | 4 +- .../storage/sqlite/SqliteIndexStorage.cpp | 124 +++++++++++------- .../data/storage/sqlite/SqliteIndexStorage.h | 6 +- src/lib/data/storage/type/StorageError.h | 31 +---- .../data/parser/cxx/CxxDiagnosticConsumer.cpp | 18 +-- src/lib_gui/qt/element/QtCodeField.cpp | 5 - src/lib_java/data/parser/java/JavaParser.cpp | 6 +- src/test/helper/TestIntermediateStorage.h | 29 ++-- 23 files changed, 290 insertions(+), 193 deletions(-) diff --git a/src/lib/data/ErrorInfo.h b/src/lib/data/ErrorInfo.h index 26a09064..9fa9a689 100644 --- a/src/lib/data/ErrorInfo.h +++ b/src/lib/data/ErrorInfo.h @@ -3,6 +3,51 @@ #include "StorageError.h" -typedef StorageError ErrorInfo; + +struct ErrorInfo +{ + ErrorInfo() + : id(0) + , message(L"") + , filePath(L"") + , lineNumber(-1) + , columnNumber(-1) + , translationUnit(L"") + , fatal(0) + , indexed(0) + {} + + ErrorInfo( + Id id, + std::wstring message, + std::wstring filePath, + uint lineNumber, + uint columnNumber, + std::wstring translationUnit, + bool fatal, + bool indexed + ) + : id(id) + , message(std::move(message)) + , filePath(std::move(filePath)) + , lineNumber(lineNumber) + , columnNumber(columnNumber) + , translationUnit(std::move(translationUnit)) + , fatal(fatal) + , indexed(indexed) + {} + + Id id; + + std::wstring message; + + std::wstring filePath; + uint lineNumber; + uint columnNumber; + + std::wstring translationUnit; + bool fatal; + bool indexed; +}; #endif // ERROR_INFO_H diff --git a/src/lib/data/indexer/TaskBuildIndex.cpp b/src/lib/data/indexer/TaskBuildIndex.cpp index b5c73cbd..764fad7d 100644 --- a/src/lib/data/indexer/TaskBuildIndex.cpp +++ b/src/lib/data/indexer/TaskBuildIndex.cpp @@ -1,17 +1,18 @@ #include "TaskBuildIndex.h" #include "AppPath.h" +#include "Blackboard.h" +#include "DialogView.h" #include "FileLogger.h" +#include "InterprocessIndexer.h" #include "MessageIndexingStatus.h" #include "MessageStatus.h" -#include "Blackboard.h" +#include "ParserClientImpl.h" +#include "StorageProvider.h" #include "TimeStamp.h" #include "UserPaths.h" #include "utilityApp.h" -#include "DialogView.h" -#include "InterprocessIndexer.h" -#include "StorageProvider.h" #if _WIN32 const std::wstring TaskBuildIndex::s_processName(L"sourcetrail_indexer.exe"); @@ -131,19 +132,22 @@ void TaskBuildIndex::doExit(std::shared_ptr blackboard) } std::vector crashedFiles = m_interprocessIndexingStatusManager.getCrashedSourceFilePaths(); - if (crashedFiles.size()) + if (!crashedFiles.empty()) { - std::shared_ptr is = std::make_shared(); + std::shared_ptr storage = std::make_shared(); + std::shared_ptr parserClient = std::make_shared(storage.get()); + for (const FilePath& path : crashedFiles) { - is->addError(StorageErrorData( + Id fileId = parserClient->recordFile(path.getCanonical(), false); + parserClient->recordError( L"The translation unit threw an exception during indexing. Please check if the source file " "conforms to the specified language standard and all necessary options are defined within your project " - "setup.", path.wstr(), 1, 1, path.wstr(), true, true - )); + "setup.", true, true, path, ParseLocation(fileId, 1, 1) + ); LOG_INFO(L"crashed translation unit: " + path.wstr()); } - m_storageProvider->insert(is); + m_storageProvider->insert(storage); } blackboard->set("indexer_threads_stopped", true); diff --git a/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.cpp b/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.cpp index 49cddc3e..76b64472 100644 --- a/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.cpp +++ b/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.cpp @@ -199,9 +199,9 @@ void SharedIntermediateStorage::setStorageComponentAccesses(const std::set SharedIntermediateStorage::getStorageErrors() const +std::vector SharedIntermediateStorage::getStorageErrors() const { - std::vector result; + std::vector result; result.reserve(m_storageErrors.size()); for (unsigned int i = 0; i < m_storageErrors.size(); i++) @@ -212,7 +212,7 @@ std::vector SharedIntermediateStorage::getStorageErrors() cons return result; } -void SharedIntermediateStorage::setStorageErrors(const std::vector& errors) +void SharedIntermediateStorage::setStorageErrors(const std::vector& errors) { m_storageErrors.clear(); diff --git a/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.h b/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.h index 7b5196a0..415f4202 100644 --- a/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.h +++ b/src/lib/data/indexer/interprocess/shared_types/SharedIntermediateStorage.h @@ -37,8 +37,8 @@ public: std::set getStorageComponentAccesses() const; void setStorageComponentAccesses(const std::set& storageComponentAccesses); - std::vector getStorageErrors() const; - void setStorageErrors(const std::vector& errors); + std::vector getStorageErrors() const; + void setStorageErrors(const std::vector& errors); Id getNextId() const; void setNextId(const Id nextId); @@ -52,7 +52,7 @@ private: SharedMemory::Vector m_storageEdges; SharedMemory::Vector m_storageLocalSymbols; SharedMemory::Vector m_storageSourceLocations; - SharedMemory::Vector m_storageErrors; + SharedMemory::Vector m_storageErrors; SharedMemory::Allocator* m_allocator; diff --git a/src/lib/data/indexer/interprocess/shared_types/SharedStorageTypes.h b/src/lib/data/indexer/interprocess/shared_types/SharedStorageTypes.h index a570f562..ef0f4bda 100644 --- a/src/lib/data/indexer/interprocess/shared_types/SharedStorageTypes.h +++ b/src/lib/data/indexer/interprocess/shared_types/SharedStorageTypes.h @@ -112,58 +112,46 @@ inline StorageLocalSymbol fromShared(const SharedStorageLocalSymbol& symbol) } -struct SharedStorageErrorData +struct SharedStorageError { - SharedStorageErrorData( + SharedStorageError( + Id id, const std::string& message, - const std::string& filePath, - uint lineNumber, - uint columnNumber, - const std::string& sourceFilePath, + const std::string& translationUnit, bool fatal, bool indexed, SharedMemory::Allocator* allocator ) - : message(message.c_str(), allocator) - , filePath(filePath.c_str(), allocator) - , lineNumber(lineNumber) - , columnNumber(columnNumber) - , translationUnit(sourceFilePath.c_str(), allocator) + : id(id) + , message(message.c_str(), allocator) + , translationUnit(translationUnit.c_str(), allocator) , fatal(fatal) , indexed(indexed) {} + Id id; SharedMemory::String message; - - SharedMemory::String filePath; - uint lineNumber; - uint columnNumber; - SharedMemory::String translationUnit; bool fatal; bool indexed; }; -inline SharedStorageErrorData toShared(const StorageErrorData& error, SharedMemory::Allocator* allocator) +inline SharedStorageError toShared(const StorageError& error, SharedMemory::Allocator* allocator) { - return SharedStorageErrorData( + return SharedStorageError( + error.id, utility::encodeToUtf8(error.message), - utility::encodeToUtf8(error.filePath), - error.lineNumber, - error.columnNumber, utility::encodeToUtf8(error.translationUnit), error.fatal, error.indexed, allocator ); } -inline StorageErrorData fromShared(const SharedStorageErrorData& error) +inline StorageError fromShared(const SharedStorageError& error) { - return StorageErrorData( + return StorageError( + error.id, utility::decodeFromUtf8(error.message.c_str()), - utility::decodeFromUtf8(error.filePath.c_str()), - error.lineNumber, - error.columnNumber, utility::decodeFromUtf8(error.translationUnit.c_str()), error.fatal, error.indexed diff --git a/src/lib/data/location/SourceLocationFile.cpp b/src/lib/data/location/SourceLocationFile.cpp index 5beab84a..9c8ac9d3 100644 --- a/src/lib/data/location/SourceLocationFile.cpp +++ b/src/lib/data/location/SourceLocationFile.cpp @@ -211,6 +211,28 @@ std::shared_ptr SourceLocationFile::getFilteredByType(Locati return ret; } +std::shared_ptr SourceLocationFile::getFilteredByTypes(const std::vector& types) const +{ + size_t typeMask = 0; + for (LocationType type : types) + { + typeMask |= 1 << type; + } + + std::shared_ptr ret = + std::make_shared(getFilePath(), false, isComplete(), isIndexed()); + + for (const std::shared_ptr& location : m_locations) + { + if ((1 << location->getType()) & typeMask) + { + ret->addSourceLocationCopy(location.get()); + } + } + + return ret; +} + std::wostream& operator<<(std::wostream& ostream, const SourceLocationFile& file) { ostream << L"file \"" << file.getFilePath().wstr() << L"\""; diff --git a/src/lib/data/location/SourceLocationFile.h b/src/lib/data/location/SourceLocationFile.h index da042634..fad5d84c 100644 --- a/src/lib/data/location/SourceLocationFile.h +++ b/src/lib/data/location/SourceLocationFile.h @@ -58,6 +58,7 @@ public: std::shared_ptr getFilteredByLines(size_t firstLineNumber, size_t lastLineNumber) const; std::shared_ptr getFilteredByType(LocationType type) const; + std::shared_ptr getFilteredByTypes(const std::vector& types) const; private: const FilePath m_filePath; diff --git a/src/lib/data/parser/ParserClient.h b/src/lib/data/parser/ParserClient.h index e42aab50..5470a66d 100644 --- a/src/lib/data/parser/ParserClient.h +++ b/src/lib/data/parser/ParserClient.h @@ -30,8 +30,7 @@ public: virtual void recordLocation(Id elementId, const ParseLocation& location, ParseLocationType type) = 0; virtual void recordComment(const ParseLocation& location) = 0; - virtual void recordError(const FilePath& filePath, uint lineNumber, uint columnNumber, const std::wstring& message, - bool fatal, bool indexed, const FilePath& translationUnit) = 0; + virtual void recordError(const std::wstring& message, bool fatal, bool indexed, const FilePath& translationUnit, const ParseLocation& location) = 0; }; #endif // PARSER_CLIENT_H diff --git a/src/lib/data/parser/ParserClientImpl.cpp b/src/lib/data/parser/ParserClientImpl.cpp index 9263f069..bd72c502 100644 --- a/src/lib/data/parser/ParserClientImpl.cpp +++ b/src/lib/data/parser/ParserClientImpl.cpp @@ -78,20 +78,19 @@ void ParserClientImpl::recordComment(const ParseLocation& location) } void ParserClientImpl::recordError( - const FilePath& filePath, uint lineNumber, uint columnNumber, const std::wstring& message, bool fatal, bool indexed, - const FilePath& translationUnit) + const std::wstring& message, bool fatal, bool indexed, + const FilePath& translationUnit, const ParseLocation& location) { - if (!filePath.empty()) + if (location.fileId != 0) { - m_storage->addError(StorageErrorData( + Id errorId = m_storage->addError(StorageErrorData( message, - filePath.wstr(), - lineNumber, - columnNumber, translationUnit.wstr(), fatal, indexed )); + + addSourceLocation(errorId, location, LOCATION_ERROR); } } diff --git a/src/lib/data/parser/ParserClientImpl.h b/src/lib/data/parser/ParserClientImpl.h index 8f16f7e8..00577cb5 100644 --- a/src/lib/data/parser/ParserClientImpl.h +++ b/src/lib/data/parser/ParserClientImpl.h @@ -28,7 +28,7 @@ public: void recordLocation(Id elementId, const ParseLocation& location, ParseLocationType type) override; void recordComment(const ParseLocation& location) override; - void recordError(const FilePath& filePath, uint lineNumber, uint columnNumber, const std::wstring& message, bool fatal, bool indexed, const FilePath& translationUnit) override; + void recordError(const std::wstring& message, bool fatal, bool indexed, const FilePath& translationUnit, const ParseLocation& location) override; private: NodeType symbolKindToNodeType(SymbolKind symbolType) const; diff --git a/src/lib/data/storage/IntermediateStorage.cpp b/src/lib/data/storage/IntermediateStorage.cpp index faa50087..ab9da909 100644 --- a/src/lib/data/storage/IntermediateStorage.cpp +++ b/src/lib/data/storage/IntermediateStorage.cpp @@ -1,5 +1,6 @@ #include "IntermediateStorage.h" +#include "LocationType.h" #include "utility.h" IntermediateStorage::IntermediateStorage() @@ -46,8 +47,8 @@ size_t IntermediateStorage::getByteSize(size_t stringSize) const for (const StorageErrorData& storageError: getErrors()) { byteSize += sizeof(StorageErrorData); - byteSize += stringSize + storageError.filePath.size(); byteSize += stringSize + storageError.message.size(); + byteSize += stringSize + storageError.translationUnit.size(); } for (const StorageNode& storageNode: getStorageNodes()) @@ -99,15 +100,18 @@ void IntermediateStorage::setAllFilesIncomplete() void IntermediateStorage::setFilesWithErrorsIncomplete() { - std::set errorFileNames; - for (const StorageErrorData& error : m_errors) + std::set errorFileIds; + for (const StorageSourceLocation& location : m_sourceLocations) { - errorFileNames.insert(error.filePath); + if (location.type == locationTypeToInt(LOCATION_ERROR)) + { + errorFileIds.insert(location.fileNodeId); + } } for (StorageFile& file : m_files) { - if (errorFileNames.find(file.filePath) != errorFileNames.end()) + if (errorFileIds.find(file.id) != errorFileIds.end()) { file.complete = false; } @@ -281,13 +285,18 @@ void IntermediateStorage::addComponentAccesses(const std::vectorsecond].id; } + + Id errorId = m_nextId++; + m_errors.emplace_back(errorId, errorData); + m_errorsIndex.emplace(errorData, m_errors.size() - 1); + return errorId; } const std::vector& IntermediateStorage::getStorageNodes() const @@ -330,7 +339,7 @@ const std::set& IntermediateStorage::getComponentAccesse return m_componentAccesses; } -const std::vector& IntermediateStorage::getErrors() const +const std::vector& IntermediateStorage::getErrors() const { return m_errors; } @@ -395,10 +404,15 @@ void IntermediateStorage::setComponentAccesses(std::set m_componentAccesses = std::move(componentAccesses); } -void IntermediateStorage::setErrors(std::vector errors) +void IntermediateStorage::setErrors(std::vector errors) { m_errors = std::move(errors); - m_errorsIndex = utility::toSet(m_errors); + + m_errorsIndex.clear(); + for (size_t i = 0; i < m_errors.size(); i++) + { + m_errorsIndex.emplace(m_errors[i], i); + } } Id IntermediateStorage::getNextId() const diff --git a/src/lib/data/storage/IntermediateStorage.h b/src/lib/data/storage/IntermediateStorage.h index f441e04a..7faf6209 100644 --- a/src/lib/data/storage/IntermediateStorage.h +++ b/src/lib/data/storage/IntermediateStorage.h @@ -38,7 +38,7 @@ public: void addOccurrences(const std::vector& occurrences) override; void addComponentAccess(const StorageComponentAccess& componentAccess) override; void addComponentAccesses(const std::vector& componentAccesses) override; - void addError(const StorageErrorData& errorData) override; + Id addError(const StorageErrorData& errorData) override; const std::vector& getStorageNodes() const override; const std::vector& getStorageFiles() const override; @@ -48,7 +48,7 @@ public: const std::set& getStorageSourceLocations() const override; const std::set& getStorageOccurrences() const override; const std::set& getComponentAccesses() const override; - const std::vector& getErrors() const override; + const std::vector& getErrors() const override; void setStorageNodes(std::vector storageNodes); void setStorageFiles(std::vector storageFiles); @@ -58,14 +58,12 @@ public: void setStorageSourceLocations(std::set storageSourceLocations); void setStorageOccurrences(std::set storageOccurrences); void setComponentAccesses(std::set componentAccesses); - void setErrors(std::vector errors); + void setErrors(std::vector errors); Id getNextId() const; void setNextId(const Id nextId); private: - std::wstring serialize(const StorageErrorData& errorData) const; - std::map m_nodesIndex; std::map m_nodeIdIndex; std::vector m_nodes; @@ -86,8 +84,8 @@ private: std::set m_componentAccesses; - std::set m_errorsIndex; // this is used to prevent duplicates (unique) - std::vector m_errors; + std::map m_errorsIndex; // this is used to prevent duplicates (unique) + std::vector m_errors; Id m_nextId; }; diff --git a/src/lib/data/storage/PersistentStorage.cpp b/src/lib/data/storage/PersistentStorage.cpp index e3d0491c..d6a3b114 100644 --- a/src/lib/data/storage/PersistentStorage.cpp +++ b/src/lib/data/storage/PersistentStorage.cpp @@ -138,9 +138,9 @@ void PersistentStorage::addComponentAccesses(const std::vector& PersistentStorage::getStorageNodes() const @@ -183,9 +183,9 @@ const std::set& PersistentStorage::getComponentAccesses( return m_storageData.accesses = utility::toSet(m_sqliteIndexStorage.getAll()); } -const std::vector& PersistentStorage::getErrors() const +const std::vector& PersistentStorage::getErrors() const { - std::vector errors; + std::vector errors; for (const StorageError& error : m_sqliteIndexStorage.getAll()) { errors.emplace_back(error); @@ -204,7 +204,7 @@ void PersistentStorage::finishInjection() { m_sqliteIndexStorage.commitTransaction(); - std::vector errors = m_sqliteIndexStorage.getAll(); + std::vector errors = m_sqliteIndexStorage.getAllErrorInfos(); if (m_preInjectionErrorCount < errors.size()) { ErrorCountInfo errorCount(errors); @@ -333,7 +333,6 @@ void PersistentStorage::clearFileElements(const std::vector& filePaths m_sqliteIndexStorage.beginTransaction(); m_sqliteIndexStorage.removeElementsWithLocationInFiles(fileNodeIds, updateStatusCallback); m_sqliteIndexStorage.removeElements(fileNodeIds); - m_sqliteIndexStorage.removeErrorsInFiles(filePaths); m_sqliteIndexStorage.commitTransaction(); updateStatusCallback(100); } @@ -1344,7 +1343,7 @@ std::shared_ptr PersistentStorage::getSourceLocationsF for (const StorageSourceLocation& sourceLocation: m_sqliteIndexStorage.getAllByIds(locationIds)) { const LocationType type = intToLocationType(sourceLocation.type); - if (type == LOCATION_QUALIFIER || type == LOCATION_SIGNATURE) + if (type != LOCATION_TOKEN && type != LOCATION_SCOPE && type != LOCATION_QUALIFIER && type != LOCATION_LOCAL_SYMBOL) { continue; } @@ -1407,6 +1406,12 @@ std::shared_ptr PersistentStorage::getSourceLocationsF elementIds.push_back(occurrence.elementId); } + const LocationType type = intToLocationType(location.type); + if (type != LOCATION_TOKEN && type != LOCATION_SCOPE && type != LOCATION_QUALIFIER && type != LOCATION_LOCAL_SYMBOL) + { + continue; + } + collection->addSourceLocation( intToLocationType(location.type), location.id, @@ -1428,7 +1433,9 @@ std::shared_ptr PersistentStorage::getSourceLocationsForFile { TRACE(); - return m_sqliteIndexStorage.getSourceLocationsForFile(filePath); + return m_sqliteIndexStorage.getSourceLocationsForFile(filePath)->getFilteredByTypes({ + LOCATION_TOKEN, LOCATION_SCOPE, LOCATION_QUALIFIER, LOCATION_LOCAL_SYMBOL + }); } std::shared_ptr PersistentStorage::getSourceLocationsForLinesInFile( @@ -1438,7 +1445,9 @@ std::shared_ptr PersistentStorage::getSourceLocationsForLine TRACE(); return m_sqliteIndexStorage.getSourceLocationsForLinesInFile( - filePath, startLine, endLine)->getFilteredByLines(startLine, endLine); + filePath, startLine, endLine)->getFilteredByLines(startLine, endLine)->getFilteredByTypes({ + LOCATION_TOKEN, LOCATION_SCOPE, LOCATION_QUALIFIER, LOCATION_LOCAL_SYMBOL + }); } std::shared_ptr PersistentStorage::getSourceLocationsOfTypeInFile( @@ -1515,14 +1524,14 @@ StorageStats PersistentStorage::getStorageStats() const ErrorCountInfo PersistentStorage::getErrorCount() const { - return ErrorCountInfo(m_sqliteIndexStorage.getAll()); + return ErrorCountInfo(m_sqliteIndexStorage.getAllErrorInfos()); } std::vector PersistentStorage::getErrorsLimited(const ErrorFilter& filter) const { std::vector errors; - for (const ErrorInfo& error : m_sqliteIndexStorage.getAll()) + for (const ErrorInfo& error : m_sqliteIndexStorage.getAllErrorInfos()) { if (filter.filter(error)) { @@ -1562,8 +1571,8 @@ std::vector PersistentStorage::getErrorsForFileLimited(const ErrorFil std::vector res; - std::vector errors = m_sqliteIndexStorage.getAll(); - for (const StorageError& error : errors) + std::vector errors = m_sqliteIndexStorage.getAllErrorInfos(); + for (const ErrorInfo& error : errors) { if (filter.filter(error) && fileIds.find(getFileNodeId(FilePath(error.filePath))) != fileIds.end()) { diff --git a/src/lib/data/storage/PersistentStorage.h b/src/lib/data/storage/PersistentStorage.h index 83440c90..473e40d9 100644 --- a/src/lib/data/storage/PersistentStorage.h +++ b/src/lib/data/storage/PersistentStorage.h @@ -34,7 +34,7 @@ public: void addOccurrences(const std::vector& occurrences) override; void addComponentAccess(const StorageComponentAccess& componentAccess) override; void addComponentAccesses(const std::vector& componentAccesses) override; - void addError(const StorageErrorData& data) override; + Id addError(const StorageErrorData& data) override; const std::vector& getStorageNodes() const override; const std::vector& getStorageFiles() const override; @@ -44,7 +44,7 @@ public: const std::set& getStorageSourceLocations() const override; const std::set& getStorageOccurrences() const override; const std::set& getComponentAccesses() const override; - const std::vector& getErrors() const override; + const std::vector& getErrors() const override; void startInjection() override; void finishInjection() override; @@ -164,7 +164,7 @@ private: std::set locations; std::set occurrences; std::set accesses; - std::vector errors; + std::vector errors; } m_storageData; Id getFileNodeId(const FilePath& filePath) const; diff --git a/src/lib/data/storage/Storage.cpp b/src/lib/data/storage/Storage.cpp index 40c2894f..af36b47e 100644 --- a/src/lib/data/storage/Storage.cpp +++ b/src/lib/data/storage/Storage.cpp @@ -20,9 +20,10 @@ void Storage::inject(Storage* injected) { // TRACE("inject errors"); - for (const StorageErrorData& error : injected->getErrors()) + for (const StorageError& error : injected->getErrors()) { - addError(error); + Id errorId = addError(error); + injectedIdToOwnElementId.emplace(error.id, errorId); } } diff --git a/src/lib/data/storage/Storage.h b/src/lib/data/storage/Storage.h index cdbaf391..b14fc55d 100644 --- a/src/lib/data/storage/Storage.h +++ b/src/lib/data/storage/Storage.h @@ -38,7 +38,7 @@ public: virtual void addOccurrences(const std::vector& occurrences) = 0; virtual void addComponentAccess(const StorageComponentAccess& componentAccess) = 0; virtual void addComponentAccesses(const std::vector& componentAccesses) = 0; - virtual void addError(const StorageErrorData& data) = 0; + virtual Id addError(const StorageErrorData& data) = 0; virtual const std::vector& getStorageNodes() const = 0; virtual const std::vector& getStorageFiles() const = 0; @@ -48,7 +48,7 @@ public: virtual const std::set& getStorageSourceLocations() const = 0; virtual const std::set& getStorageOccurrences() const = 0; virtual const std::set& getComponentAccesses() const = 0; - virtual const std::vector& getErrors() const = 0; + virtual const std::vector& getErrors() const = 0; void inject(Storage* injected); diff --git a/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp b/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp index 77becc7b..5e1de877 100644 --- a/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp +++ b/src/lib/data/storage/sqlite/SqliteIndexStorage.cpp @@ -4,14 +4,14 @@ #include #include "FileSystem.h" +#include "LocationType.h" #include "logging.h" #include "TextAccess.h" -#include "utilityString.h" - #include "SourceLocationCollection.h" #include "SourceLocationFile.h" +#include "utilityString.h" -const size_t SqliteIndexStorage::s_storageVersion = 20; +const size_t SqliteIndexStorage::s_storageVersion = 21; namespace { @@ -426,9 +426,6 @@ StorageError SqliteIndexStorage::addError(const StorageErrorData& data) { m_checkErrorExistsStmt.bind(1, utility::encodeToUtf8(sanitizedMessage).c_str()); m_checkErrorExistsStmt.bind(2, int(data.fatal)); - m_checkErrorExistsStmt.bind(3, utility::encodeToUtf8(data.filePath).c_str()); - m_checkErrorExistsStmt.bind(4, int(data.lineNumber)); - m_checkErrorExistsStmt.bind(5, int(data.columnNumber)); CppSQLite3Query checkQuery = executeQuery(m_checkErrorExistsStmt); if (!checkQuery.eof() && checkQuery.numFields() > 0) @@ -440,13 +437,14 @@ StorageError SqliteIndexStorage::addError(const StorageErrorData& data) if (id == 0) { - m_insertErrorStmt.bind(1, utility::encodeToUtf8(sanitizedMessage).c_str()); - m_insertErrorStmt.bind(2, data.fatal); - m_insertErrorStmt.bind(3, data.indexed); - m_insertErrorStmt.bind(4, utility::encodeToUtf8(data.filePath).c_str()); - m_insertErrorStmt.bind(5, int(data.lineNumber)); - m_insertErrorStmt.bind(6, int(data.columnNumber)); - m_insertErrorStmt.bind(7, utility::encodeToUtf8(data.translationUnit).c_str()); + executeStatement(m_insertElementStmt); + id = m_database.lastRowId(); + + m_insertErrorStmt.bind(1, int(id)); + m_insertErrorStmt.bind(2, utility::encodeToUtf8(sanitizedMessage).c_str()); + m_insertErrorStmt.bind(3, data.fatal); + m_insertErrorStmt.bind(4, data.indexed); + m_insertErrorStmt.bind(5, utility::encodeToUtf8(data.translationUnit).c_str()); const bool success = executeStatement(m_insertErrorStmt); if (success) @@ -624,13 +622,6 @@ void SqliteIndexStorage::removeAllErrors() ); } -void SqliteIndexStorage::removeErrorsInFiles(const std::vector& filePaths) -{ - executeStatement( - "DELETE FROM error WHERE file_path IN ('" + utility::join(utility::toStrings(filePaths), "', '") + "');" - ); -} - bool SqliteIndexStorage::isEdge(Id elementId) const { int count = executeStatementScalar("SELECT count(*) FROM edge WHERE id = " + std::to_string(elementId) + ";", 0); @@ -816,7 +807,7 @@ void SqliteIndexStorage::setFileIndexed(Id fileId, bool indexed) void SqliteIndexStorage::setFileCompleteIfNoError(Id fileId, const std::wstring& filePath, bool complete) { - bool fileHasErrors = doGetFirst("WHERE file_path == '" + utility::encodeToUtf8(filePath) + "'").id; + bool fileHasErrors = doGetFirst("WHERE file_node_id == " + std::to_string(fileId) + " AND type == " + std::to_string(locationTypeToInt(LOCATION_ERROR))).id; if (fileHasErrors != complete) { executeStatement( @@ -970,6 +961,58 @@ std::vector SqliteIndexStorage::getComponentAccessesByNo return doGetAll("WHERE node_id IN (" + utility::join(utility::toStrings(nodeIds), ',') + ")"); } +std::vector SqliteIndexStorage::getAllErrorInfos() const +{ + std::vector errorInfos; + + CppSQLite3Query q = executeQuery( + "SELECT error.id, error.message, error.fatal, error.indexed, error.translation_unit, file.path, source_location.start_line, source_location.start_column " + "FROM occurrence " + "INNER JOIN error ON (error.id = occurrence.element_id) " + "INNER JOIN source_location ON (source_location.id = occurrence.source_location_id) " + "INNER JOIN file ON (file.id = source_location.file_node_id);" + ); + + std::map errorIdCount; + + while (!q.eof()) + { + const Id id = q.getIntField(0, 0); + const std::string message = q.getStringField(1, ""); + const bool fatal = q.getIntField(2, 0); + const bool indexed = q.getIntField(3, 0); + const std::string translationUnit = q.getStringField(4, ""); + const std::string filePath = q.getStringField(5, ""); + const int lineNumber = q.getIntField(6, -1); + const int columnNumber = q.getIntField(7, -1); + + if (id != 0) + { + // There can be multiple errors with the same id, so a count is added to the id + Id errorId = id * 10000; + auto it = errorIdCount.find(id); + if (it != errorIdCount.end()) + { + errorId += it->second; + it->second = it->second + 1; + } + else + { + errorIdCount.emplace(id, 1); + } + + errorInfos.push_back(ErrorInfo( + errorId, utility::decodeFromUtf8(message), utility::decodeFromUtf8(filePath), + lineNumber, columnNumber, utility::decodeFromUtf8(translationUnit), fatal, indexed + )); + } + + q.nextRow(); + } + + return errorInfos; +} + int SqliteIndexStorage::getNodeCount() const { return executeStatementScalar("SELECT COUNT(*) FROM node;", 0); @@ -1002,7 +1045,7 @@ int SqliteIndexStorage::getSourceLocationCount() const int SqliteIndexStorage::getErrorCount() const { - return executeStatementScalar("SELECT COUNT(*) FROM error;", 0); + return executeStatementScalar("SELECT COUNT(*) FROM error INNER JOIN occurrence ON (error.id = occurrence.element_id);", 0); } std::vector> SqliteIndexStorage::getIndices() const @@ -1026,7 +1069,7 @@ std::vector> SqliteIndexStorage::getIndices( )); indices.push_back(std::make_pair( STORAGE_MODE_WRITE, - SqliteDatabaseIndex("error_all_data_index", "error(message, fatal, file_path, line_number, column_number)") + SqliteDatabaseIndex("error_all_data_index", "error(message, fatal)") )); indices.push_back(std::make_pair( STORAGE_MODE_WRITE, @@ -1170,11 +1213,9 @@ void SqliteIndexStorage::setupTables() "message TEXT, " "fatal INTEGER NOT NULL, " "indexed INTEGER NOT NULL, " - "file_path TEXT, " - "line_number INTEGER, " - "column_number INTEGER, " "translation_unit TEXT, " - "PRIMARY KEY(id));" + "PRIMARY KEY(id), " + "FOREIGN KEY(id) REFERENCES element(id) ON DELETE CASCADE);" ); } catch (CppSQLite3Exception& e) @@ -1279,15 +1320,12 @@ void SqliteIndexStorage::setupPrecompiledStatements() m_checkErrorExistsStmt = m_database.compileStatement( "SELECT id FROM error WHERE " "message = ? AND " - "fatal == ? AND " - "file_path == ? AND " - "line_number == ? AND " - "column_number == ? " + "fatal == ? " "LIMIT 1;" ); m_insertErrorStmt = m_database.compileStatement( - "INSERT INTO error(message, fatal, indexed, file_path, line_number, column_number, translation_unit) " - "VALUES(?, ?, ?, ?, ?, ?, ?);" + "INSERT INTO error(id, message, fatal, indexed, translation_unit) " + "VALUES(?, ?, ?, ?, ?);" ); } catch (CppSQLite3Exception& e) @@ -1483,27 +1521,23 @@ template <> void SqliteIndexStorage::forEach(const std::string& query, std::function func) const { CppSQLite3Query q = executeQuery( - "SELECT message, fatal, indexed, file_path, line_number, column_number, translation_unit FROM error " + query + ";" + "SELECT id, message, fatal, indexed, translation_unit FROM error " + query + ";" ); - Id id = 1; while (!q.eof()) { - const std::string message = q.getStringField(0, ""); - const bool fatal = q.getIntField(1, 0); - const bool indexed = q.getIntField(2, 0); - const std::string filePath = q.getStringField(3, ""); - const int lineNumber = q.getIntField(4, -1); - const int columnNumber = q.getIntField(5, -1); - const std::string translationUnit = q.getStringField(6, ""); + const Id id = q.getIntField(0, 0); + const std::string message = q.getStringField(1, ""); + const bool fatal = q.getIntField(2, 0); + const bool indexed = q.getIntField(3, 0); + const std::string translationUnit = q.getStringField(4, ""); - if (lineNumber != -1 && columnNumber != -1) + if (id != 0) { func(StorageError( - id, utility::decodeFromUtf8(message), utility::decodeFromUtf8(filePath), lineNumber, columnNumber, + id, utility::decodeFromUtf8(message), utility::decodeFromUtf8(translationUnit), fatal, indexed )); - id++; } q.nextRow(); diff --git a/src/lib/data/storage/sqlite/SqliteIndexStorage.h b/src/lib/data/storage/sqlite/SqliteIndexStorage.h index 11240a34..e2278b5e 100644 --- a/src/lib/data/storage/sqlite/SqliteIndexStorage.h +++ b/src/lib/data/storage/sqlite/SqliteIndexStorage.h @@ -5,7 +5,9 @@ #include #include +#include "ErrorInfo.h" #include "LocationType.h" +#include "LowMemoryStringMap.h" #include "SqliteDatabaseIndex.h" #include "SqliteStorage.h" #include "StorageComponentAccess.h" @@ -17,7 +19,6 @@ #include "StorageOccurrence.h" #include "StorageSourceLocation.h" #include "StorageSymbol.h" -#include "LowMemoryStringMap.h" #include "types.h" #include "utility.h" #include "utilityString.h" @@ -69,7 +70,6 @@ public: void removeElementsWithLocationInFiles(const std::vector& fileIds, std::function updateStatusCallback); void removeAllErrors(); - void removeErrorsInFiles(const std::vector& filePaths); bool isEdge(Id elementId) const; bool isNode(Id elementId) const; @@ -119,6 +119,8 @@ public: StorageComponentAccess getComponentAccessByNodeId(Id memberEdgeId) const; std::vector getComponentAccessesByNodeIds(const std::vector& memberEdgeIds) const; + std::vector getAllErrorInfos() const; + template std::vector getAll() const { diff --git a/src/lib/data/storage/type/StorageError.h b/src/lib/data/storage/type/StorageError.h index 49ae6194..a36cfe19 100644 --- a/src/lib/data/storage/type/StorageError.h +++ b/src/lib/data/storage/type/StorageError.h @@ -10,27 +10,19 @@ struct StorageErrorData { StorageErrorData() : message(L"") - , filePath(L"") - , lineNumber(-1) - , columnNumber(-1) , translationUnit(L"") , fatal(0) , indexed(0) + {} StorageErrorData( std::wstring message, - std::wstring filePath, - uint lineNumber, - uint columnNumber, std::wstring translationUnit, bool fatal, bool indexed ) : message(std::move(message)) - , filePath(std::move(filePath)) - , lineNumber(lineNumber) - , columnNumber(columnNumber) , translationUnit(std::move(translationUnit)) , fatal(fatal) , indexed(indexed) @@ -42,26 +34,21 @@ struct StorageErrorData { return message < other.message; } - else if (filePath != other.filePath) + else if (translationUnit != other.translationUnit) { - return filePath < other.filePath; + return translationUnit < other.translationUnit; } - else if (lineNumber != other.lineNumber) + else if (fatal != other.fatal) { - return lineNumber < other.lineNumber; + return fatal < other.fatal; } else { - return columnNumber < other.columnNumber; + return indexed < other.indexed; } } std::wstring message; - - std::wstring filePath; - uint lineNumber; - uint columnNumber; - std::wstring translationUnit; bool fatal; bool indexed; @@ -82,18 +69,12 @@ struct StorageError: public StorageErrorData StorageError( Id id, std::wstring message, - std::wstring filePath, - uint lineNumber, - uint columnNumber, std::wstring translationUnit, bool fatal, bool indexed ) : StorageErrorData( std::move(message), - std::move(filePath), - lineNumber, - columnNumber, std::move(translationUnit), fatal, indexed diff --git a/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp b/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp index 741b81fb..7a4ce71b 100644 --- a/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp +++ b/src/lib_cxx/data/parser/cxx/CxxDiagnosticConsumer.cpp @@ -63,6 +63,7 @@ void CxxDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level lev return; } + Id fileId = 0; FilePath filePath; uint lineNumber = 0; uint columnNumber = 0; @@ -76,13 +77,14 @@ void CxxDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level lev loc = info.getLocation(); } - clang::FileID fileId = sourceManager.getFileID(loc); - const clang::FileEntry* fileEntry = sourceManager.getFileEntryForID(fileId); + clang::FileID clangFileId = sourceManager.getFileID(loc); + const clang::FileEntry* fileEntry = sourceManager.getFileEntryForID(clangFileId); if (fileEntry != nullptr && fileEntry->isValid()) { ParseLocation location = utility::getParseLocation(loc, sourceManager, nullptr, m_canonicalFilePathCache); - filePath = m_canonicalFilePathCache->getCanonicalFilePath(location.fileId); + fileId = location.fileId; + filePath = m_canonicalFilePathCache->getCanonicalFilePath(fileId); lineNumber = location.startLineNumber; columnNumber = location.startColumnNumber; } @@ -92,6 +94,7 @@ void CxxDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level lev if (fileEntry != nullptr && fileEntry->isValid()) { filePath = m_canonicalFilePathCache->getCanonicalFilePath(fileEntry); + fileId = m_client->recordFile(filePath, false /*keeps the "indexed" state if the file already exists*/); lineNumber = 1; columnNumber = 1; } @@ -100,20 +103,19 @@ void CxxDiagnosticConsumer::HandleDiagnostic(clang::DiagnosticsEngine::Level lev else { filePath = m_sourceFilePath; + fileId = m_client->recordFile(filePath, false /*keeps the "indexed" state if the file already exists*/); lineNumber = 1; columnNumber = 1; } - if (!filePath.empty()) + if (fileId != 0) { m_client->recordError( - filePath, - lineNumber, - columnNumber, utility::decodeFromUtf8(message), level == clang::DiagnosticsEngine::Fatal, m_canonicalFilePathCache->getFileRegister()->hasFilePath(filePath), - m_sourceFilePath + m_sourceFilePath, + ParseLocation(fileId, lineNumber, columnNumber) ); } } diff --git a/src/lib_gui/qt/element/QtCodeField.cpp b/src/lib_gui/qt/element/QtCodeField.cpp index ceae0f21..92630252 100644 --- a/src/lib_gui/qt/element/QtCodeField.cpp +++ b/src/lib_gui/qt/element/QtCodeField.cpp @@ -375,11 +375,6 @@ void QtCodeField::createAnnotations(std::shared_ptr location locationFile->forEachSourceLocation( [&](const SourceLocation* location) { - if (location->getType() == LOCATION_SIGNATURE || location->getType() == LOCATION_COMMENT) - { - return; - } - if (location->getLocationId() && locationIds.find(location->getLocationId()) != locationIds.end()) { return; diff --git a/src/lib_java/data/parser/java/JavaParser.cpp b/src/lib_java/data/parser/java/JavaParser.cpp index 395ee52c..530b2009 100644 --- a/src/lib_java/data/parser/java/JavaParser.cpp +++ b/src/lib_java/data/parser/java/JavaParser.cpp @@ -264,13 +264,11 @@ void JavaParser::doRecordError( bool indexed = jIndexed; m_client->recordError( - m_currentFilePath, - beginLine, - beginColumn, utility::decodeFromUtf8(m_javaEnvironment->toStdString(jMessage)), fatal, indexed, - FilePath() + FilePath(), + ParseLocation(m_currentFileId, beginLine, beginColumn) ); } diff --git a/src/test/helper/TestIntermediateStorage.h b/src/test/helper/TestIntermediateStorage.h index 3370a704..716566eb 100644 --- a/src/test/helper/TestIntermediateStorage.h +++ b/src/test/helper/TestIntermediateStorage.h @@ -44,6 +44,7 @@ public: std::multimap signatureLocationMap; std::multimap localSymbolLocationMap; std::multimap qualifierLocationMap; + std::multimap errorLocationMap; std::vector commentLocations; for (const StorageSourceLocation& location : getStorageSourceLocations()) { @@ -53,7 +54,7 @@ public: elementIds.emplace_back(it->second); } - if (!elementIds.size()) + if (elementIds.empty()) { elementIds.emplace_back(0); } @@ -92,6 +93,12 @@ public: signatureLocationMap.emplace(elementId, location); } break; + case LOCATION_ERROR: + if (elementId) + { + errorLocationMap.emplace(elementId, location); + } + break; case LOCATION_COMMENT: commentLocations.emplace_back(location); break; @@ -262,18 +269,16 @@ public: addLine(L"COMMENT: comment" + addFileName(locStr, filePathMap[location.fileNodeId])); } - for (const StorageErrorData& error : getErrors()) + for (const StorageError& error : getErrors()) { - std::wstring locStr = addLocationStr( - L"", - StorageSourceLocation( - 0, 0, error.lineNumber, error.columnNumber, error.lineNumber, error.columnNumber, - locationTypeToInt(LOCATION_ERROR) - ) - ); - - errors.emplace_back(error.message + locStr); - addLine(L"ERROR: " + error.message + addFileName(locStr, FilePath(error.filePath))); + for (auto errorLocationIt = errorLocationMap.find(error.id); + errorLocationIt != errorLocationMap.end() && errorLocationIt->first == error.id; + errorLocationIt++) + { + std::wstring locStr = addLocationStr(L"", errorLocationIt->second); + errors.emplace_back(error.message + locStr); + addLine(L"ERROR: " + error.message + addFileName(locStr, filePathMap[errorLocationIt->second.fileNodeId])); + } } }