From fdd12cde25d8f6c0b76b4401c4752479cbd30de6 Mon Sep 17 00:00:00 2001
From: BigBahss <tdennis4496@gmail.com>
Date: Tue, 16 Feb 2021 07:15:47 -0500
Subject: [PATCH] Refactor ParseUtil to stop using pointers and
 output-parameters

---
 include/core/parseutil.h | 41 ++++++++---------
 src/core/parseutil.cpp   | 99 ++++++++++++++++++----------------------
 src/project.cpp          | 41 +++++++----------
 3 files changed, 81 insertions(+), 100 deletions(-)

diff --git a/include/core/parseutil.h b/include/core/parseutil.h
index 399f43fb..7c9b225e 100644
--- a/include/core/parseutil.h
+++ b/include/core/parseutil.h
@@ -39,31 +39,30 @@ public:
 class ParseUtil
 {
 public:
-    ParseUtil();
-    void set_root(QString);
-    static QString readTextFile(QString);
+    ParseUtil() { };
+    void set_root(const QString &dir);
+    static QString readTextFile(const QString &path);
     static int textFileLineCount(const QString &path);
-    void strip_comment(QString*);
     QList<QStringList> parseAsm(const QString &filename);
-    int evaluateDefine(QString, QMap<QString, int>*);
-    QStringList readCArray(QString text, QString label);
-    QMap<QString, QString> readNamedIndexCArray(QString text, QString label);
-    QString readCIncbin(QString text, QString label);
-    QMap<QString, int> readCDefines(QString filename, QStringList prefixes, QMap<QString, int> = QMap<QString, int>());
-    void readCDefinesSorted(QString, QStringList, QStringList*, QMap<QString, int> = QMap<QString, int>());
-    QList<QStringList> getLabelMacros(const QList<QStringList> &, const QString &);
-    QStringList getLabelValues(const QList<QStringList> &, const QString &);
-    bool tryParseJsonFile(QJsonDocument *out, QString filepath);
-    bool ensureFieldsExist(QJsonObject obj, QList<QString> fields);
+    int evaluateDefine(const QString&, const QMap<QString, int>&);
+    QStringList readCArray(const QString &text, const QString &label);
+    QMap<QString, QString> readNamedIndexCArray(const QString &text, const QString &label);
+    QString readCIncbin(const QString &text, const QString &label);
+    QMap<QString, int> readCDefines(const QString &filename, const QStringList &prefixes, QMap<QString, int> = { });
+    QStringList readCDefinesSorted(const QString&, const QStringList&, const QMap<QString, int>& = { });
+    QList<QStringList> getLabelMacros(const QList<QStringList>&, const QString&);
+    QStringList getLabelValues(const QList<QStringList>&, const QString&);
+    bool tryParseJsonFile(QJsonDocument *out, const QString &filepath);
+    bool ensureFieldsExist(const QJsonObject &obj, const QList<QString> &fields);
 
     // Returns the 1-indexed line number for the definition of scriptLabel in the scripts file at filePath.
     // Returns 0 if a definition for scriptLabel cannot be found.
     static int getScriptLineNumber(const QString &filePath, const QString &scriptLabel);
     static int getRawScriptLineNumber(QString text, const QString &scriptLabel);
     static int getPoryScriptLineNumber(QString text, const QString &scriptLabel);
-    static QString &removeStringLiterals(QString &text);
-    static QString &removeLineComments(QString &text, const QString &commentSymbol);
-    static QString &removeLineComments(QString &text, const QStringList &commentSymbols);
+    static QString removeStringLiterals(QString text);
+    static QString removeLineComments(QString text, const QString &commentSymbol);
+    static QString removeLineComments(QString text, const QStringList &commentSymbols);
 
     static QStringList splitShellCommand(QStringView command);
 
@@ -71,10 +70,10 @@ private:
     QString root;
     QString text;
     QString file;
-    QList<Token> tokenizeExpression(QString expression, QMap<QString, int>* knownIdentifiers);
-    QList<Token> generatePostfix(QList<Token> tokens);
-    int evaluatePostfix(QList<Token> postfix);
-    void error(QString message, QString expression);
+    QList<Token> tokenizeExpression(QString expression, const QMap<QString, int> &knownIdentifiers);
+    QList<Token> generatePostfix(const QList<Token> &tokens);
+    int evaluatePostfix(const QList<Token> &postfix);
+    void error(const QString &message, const QString &expression);
 };
 
 #endif // PARSEUTIL_H
diff --git a/src/core/parseutil.cpp b/src/core/parseutil.cpp
index cd053458..30ebe19c 100644
--- a/src/core/parseutil.cpp
+++ b/src/core/parseutil.cpp
@@ -6,15 +6,12 @@
 #include <QJsonObject>
 #include <QStack>
 
-ParseUtil::ParseUtil()
-{
-}
 
-void ParseUtil::set_root(QString dir) {
+void ParseUtil::set_root(const QString &dir) {
     this->root = dir;
 }
 
-void ParseUtil::error(QString message, QString expression) {
+void ParseUtil::error(const QString &message, const QString &expression) {
     QStringList lines = text.split(QRegularExpression("[\r\n]"));
     int lineNum = 0, colNum = 0;
     for (QString line : lines) {
@@ -25,21 +22,7 @@ void ParseUtil::error(QString message, QString expression) {
     logError(QString("%1:%2:%3: %4").arg(file).arg(lineNum).arg(colNum).arg(message));
 }
 
-void ParseUtil::strip_comment(QString *line) {
-    bool in_string = false;
-    for (int i = 0; i < line->length(); i++) {
-        if (line->at(i) == '"') {
-            in_string = !in_string;
-        } else if (line->at(i) == '@') {
-            if (!in_string) {
-                line->truncate(i);
-                break;
-            }
-        }
-    }
-}
-
-QString ParseUtil::readTextFile(QString path) {
+QString ParseUtil::readTextFile(const QString &path) {
     QFile file(path);
     if (!file.open(QIODevice::ReadOnly)) {
         logError(QString("Could not open '%1': ").arg(path) + file.errorString());
@@ -63,21 +46,23 @@ QList<QStringList> ParseUtil::parseAsm(const QString &filename) {
     QList<QStringList> parsed;
 
     text = readTextFile(root + '/' + filename);
-    const QStringList lines = text.split('\n');
-    for (QString line : lines) {
-        strip_comment(&line);
-        if (line.trimmed().isEmpty()) {
-        } else if (line.contains(':')) {
+    const QStringList lines = removeLineComments(text, "@").split('\n');
+    for (const auto &line : lines) {
+        const QString trimmedLine = line.trimmed();
+        if (trimmedLine.isEmpty()) {
+            continue;
+        }
+
+        if (line.contains(':')) {
             const QString label = line.left(line.indexOf(':'));
             const QStringList list{ ".label", label }; // .label is not a real keyword. It's used only to make the output more regular.
             parsed.append(list);
             // There should not be anything else on the line.
             // gas will raise a syntax error if there is.
         } else {
-            line = line.trimmed();
-            int index = line.indexOf(QRegExp("\\s+"));
-            const QString macro = line.left(index);
-            QStringList params(line.right(line.length() - index).trimmed().split(QRegExp("\\s*,\\s*")));
+            int index = trimmedLine.indexOf(QRegExp("\\s+"));
+            const QString macro = trimmedLine.left(index);
+            QStringList params(trimmedLine.right(trimmedLine.length() - index).trimmed().split(QRegExp("\\s*,\\s*")));
             params.prepend(macro);
             parsed.append(params);
         }
@@ -85,13 +70,13 @@ QList<QStringList> ParseUtil::parseAsm(const QString &filename) {
     return parsed;
 }
 
-int ParseUtil::evaluateDefine(QString define, QMap<QString, int>* knownDefines) {
+int ParseUtil::evaluateDefine(const QString &define, const QMap<QString, int> &knownDefines) {
     QList<Token> tokens = tokenizeExpression(define, knownDefines);
     QList<Token> postfixExpression = generatePostfix(tokens);
     return evaluatePostfix(postfixExpression);
 }
 
-QList<Token> ParseUtil::tokenizeExpression(QString expression, QMap<QString, int>* knownIdentifiers) {
+QList<Token> ParseUtil::tokenizeExpression(QString expression, const QMap<QString, int> &knownIdentifiers) {
     QList<Token> tokens;
 
     QStringList tokenTypes = (QStringList() << "hex" << "decimal" << "identifier" << "operator" << "leftparen" << "rightparen");
@@ -108,8 +93,8 @@ QList<Token> ParseUtil::tokenizeExpression(QString expression, QMap<QString, int
             QString token = match.captured(tokenType);
             if (!token.isEmpty()) {
                 if (tokenType == "identifier") {
-                    if (knownIdentifiers->contains(token)) {
-                        QString actualToken = QString("%1").arg(knownIdentifiers->value(token));
+                    if (knownIdentifiers.contains(token)) {
+                        QString actualToken = QString("%1").arg(knownIdentifiers.value(token));
                         expression = expression.replace(0, token.length(), actualToken);
                         token = actualToken;
                         tokenType = "decimal";
@@ -152,7 +137,7 @@ QMap<QString, int> Token::precedenceMap = QMap<QString, int>(
 
 // Shunting-yard algorithm for generating postfix notation.
 // https://en.wikipedia.org/wiki/Shunting-yard_algorithm
-QList<Token> ParseUtil::generatePostfix(QList<Token> tokens) {
+QList<Token> ParseUtil::generatePostfix(const QList<Token> &tokens) {
     QList<Token> output;
     QStack<Token> operatorStack;
     for (Token token : tokens) {
@@ -194,7 +179,7 @@ QList<Token> ParseUtil::generatePostfix(QList<Token> tokens) {
 
 // Evaluate postfix expression.
 // https://en.wikipedia.org/wiki/Reverse_Polish_notation#Postfix_evaluation_algorithm
-int ParseUtil::evaluatePostfix(QList<Token> postfix) {
+int ParseUtil::evaluatePostfix(const QList<Token> &postfix) {
     QStack<Token> stack;
     for (Token token : postfix) {
         if (token.type == TokenClass::Operator && stack.size() > 1) {
@@ -228,7 +213,7 @@ int ParseUtil::evaluatePostfix(QList<Token> postfix) {
     return stack.size() ? stack.pop().value.toInt(nullptr, 0) : 0;
 }
 
-QString ParseUtil::readCIncbin(QString filename, QString label) {
+QString ParseUtil::readCIncbin(const QString &filename, const QString &label) {
     QString path;
 
     if (label.isNull()) {
@@ -251,7 +236,10 @@ QString ParseUtil::readCIncbin(QString filename, QString label) {
     return path;
 }
 
-QMap<QString, int> ParseUtil::readCDefines(QString filename, QStringList prefixes, QMap<QString, int> allDefines) {
+QMap<QString, int> ParseUtil::readCDefines(const QString &filename,
+                                           const QStringList &prefixes,
+                                           QMap<QString, int> allDefines)
+{
     QMap<QString, int> filteredDefines;
 
     file = filename;
@@ -280,7 +268,7 @@ QMap<QString, int> ParseUtil::readCDefines(QString filename, QStringList prefixe
         QString name = match.captured("defineName");
         QString expression = match.captured("defineValue");
         if (expression == " ") continue;
-        int value = evaluateDefine(expression, &allDefines);
+        int value = evaluateDefine(expression, allDefines);
         allDefines.insert(name, value);
         for (QString prefix : prefixes) {
             if (name.startsWith(prefix) || QRegularExpression(prefix).match(name).hasMatch()) {
@@ -291,7 +279,10 @@ QMap<QString, int> ParseUtil::readCDefines(QString filename, QStringList prefixe
     return filteredDefines;
 }
 
-void ParseUtil::readCDefinesSorted(QString filename, QStringList prefixes, QStringList* definesToSet, QMap<QString, int> knownDefines) {
+QStringList ParseUtil::readCDefinesSorted(const QString &filename,
+                                          const QStringList &prefixes,
+                                          const QMap<QString, int> &knownDefines)
+{
     QMap<QString, int> defines = readCDefines(filename, prefixes, knownDefines);
 
     // The defines should to be sorted by their underlying value, not alphabetically.
@@ -300,10 +291,10 @@ void ParseUtil::readCDefinesSorted(QString filename, QStringList prefixes, QStri
     for (QString defineName : defines.keys()) {
         definesInverse.insert(defines[defineName], defineName);
     }
-    *definesToSet = definesInverse.values();
+    return definesInverse.values();
 }
 
-QStringList ParseUtil::readCArray(QString filename, QString label) {
+QStringList ParseUtil::readCArray(const QString &filename, const QString &label) {
     QStringList list;
 
     if (label.isNull()) {
@@ -329,7 +320,7 @@ QStringList ParseUtil::readCArray(QString filename, QString label) {
     return list;
 }
 
-QMap<QString, QString> ParseUtil::readNamedIndexCArray(QString filename, QString label) {
+QMap<QString, QString> ParseUtil::readNamedIndexCArray(const QString &filename, const QString &label) {
     text = readTextFile(root + "/" + filename);
     QMap<QString, QString> map;
 
@@ -388,16 +379,16 @@ QStringList ParseUtil::getLabelValues(const QList<QStringList> &list, const QStr
     return values;
 }
 
-bool ParseUtil::tryParseJsonFile(QJsonDocument *out, QString filepath) {
+bool ParseUtil::tryParseJsonFile(QJsonDocument *out, const QString &filepath) {
     QFile file(filepath);
     if (!file.open(QIODevice::ReadOnly)) {
         logError(QString("Error: Could not open %1 for reading").arg(filepath));
         return false;
     }
 
-    QByteArray data = file.readAll();
+    const QByteArray data = file.readAll();
     QJsonParseError parseError;
-    QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &parseError);
+    const QJsonDocument jsonDoc = QJsonDocument::fromJson(data, &parseError);
     file.close();
     if (parseError.error != QJsonParseError::NoError) {
         logError(QString("Error: Failed to parse json file %1: %2").arg(filepath).arg(parseError.errorString()));
@@ -408,7 +399,7 @@ bool ParseUtil::tryParseJsonFile(QJsonDocument *out, QString filepath) {
     return true;
 }
 
-bool ParseUtil::ensureFieldsExist(QJsonObject obj, QList<QString> fields) {
+bool ParseUtil::ensureFieldsExist(const QJsonObject &obj, const QList<QString> &fields) {
     for (QString field : fields) {
         if (!obj.contains(field)) {
             logError(QString("JSON object is missing field '%1'.").arg(field));
@@ -431,8 +422,8 @@ int ParseUtil::getScriptLineNumber(const QString &filePath, const QString &scrip
 }
 
 int ParseUtil::getRawScriptLineNumber(QString text, const QString &scriptLabel) {
-    removeStringLiterals(text);
-    removeLineComments(text, "@");
+    text = removeStringLiterals(text);
+    text = removeLineComments(text, "@");
 
     static const QRegularExpression re_incScriptLabel("\\b(?<label>[\\w_][\\w\\d_]*):{1,2}");
     QRegularExpressionMatchIterator it = re_incScriptLabel.globalMatch(text);
@@ -446,8 +437,8 @@ int ParseUtil::getRawScriptLineNumber(QString text, const QString &scriptLabel)
 }
 
 int ParseUtil::getPoryScriptLineNumber(QString text, const QString &scriptLabel) {
-    removeStringLiterals(text);
-    removeLineComments(text, {"//", "#"});
+    text = removeStringLiterals(text);
+    text = removeLineComments(text, {"//", "#"});
 
     static const QRegularExpression re_poryScriptLabel("\\b(script)(\\((global|local)\\))?\\s*\\b(?<label>[\\w_][\\w\\d_]*)");
     QRegularExpressionMatchIterator it = re_poryScriptLabel.globalMatch(text);
@@ -469,19 +460,19 @@ int ParseUtil::getPoryScriptLineNumber(QString text, const QString &scriptLabel)
     return 0;
 }
 
-QString &ParseUtil::removeStringLiterals(QString &text) {
+QString ParseUtil::removeStringLiterals(QString text) {
     static const QRegularExpression re_string("\".*\"");
     return text.remove(re_string);
 }
 
-QString &ParseUtil::removeLineComments(QString &text, const QString &commentSymbol) {
+QString ParseUtil::removeLineComments(QString text, const QString &commentSymbol) {
     const QRegularExpression re_lineComment(commentSymbol + "+.*");
     return text.remove(re_lineComment);
 }
 
-QString &ParseUtil::removeLineComments(QString &text, const QStringList &commentSymbols) {
+QString ParseUtil::removeLineComments(QString text, const QStringList &commentSymbols) {
     for (const auto &commentSymbol : commentSymbols)
-        removeLineComments(text, commentSymbol);
+        text = removeLineComments(text, commentSymbol);
     return text;
 }
 
diff --git a/src/project.cpp b/src/project.cpp
index 7058cb95..0eb61aba 100644
--- a/src/project.cpp
+++ b/src/project.cpp
@@ -164,7 +164,7 @@ QMap<QString, bool> Project::getTopLevelMapFields() {
         topLevelMapFields.insert("allow_escaping", true);
         topLevelMapFields.insert("allow_running", true);
     }
-    
+
     if (projectConfig.getFloorNumberEnabled()) {
         topLevelMapFields.insert("floor_number", true);
     }
@@ -2112,11 +2112,10 @@ bool Project::readHealLocations() {
 }
 
 bool Project::readItemNames() {
-    itemNames.clear();
     QStringList prefixes("\\bITEM_(?!(B_)?USE_)");  // Exclude ITEM_USE_ and ITEM_B_USE_ constants
     QString filename = "include/constants/items.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &itemNames);
+    itemNames = parser.readCDefinesSorted(filename, prefixes);
     if (itemNames.isEmpty()) {
         logError(QString("Failed to read item constants from %1").arg(filename));
         return false;
@@ -2131,11 +2130,10 @@ bool Project::readFlagNames() {
     fileWatcher.addPath(root + "/" + opponentsFilename);
     QMap<QString, int> maxTrainers = parser.readCDefines(opponentsFilename, QStringList() << "\\bMAX_");
     // Parse flags
-    flagNames.clear();
     QStringList prefixes("\\bFLAG_");
     QString flagsFilename = "include/constants/flags.h";
     fileWatcher.addPath(root + "/" + flagsFilename);
-    parser.readCDefinesSorted(flagsFilename, prefixes, &flagNames, maxTrainers);
+    flagNames = parser.readCDefinesSorted(flagsFilename, prefixes, maxTrainers);
     if (flagNames.isEmpty()) {
         logError(QString("Failed to read flag constants from %1").arg(flagsFilename));
         return false;
@@ -2144,11 +2142,10 @@ bool Project::readFlagNames() {
 }
 
 bool Project::readVarNames() {
-    varNames.clear();
     QStringList prefixes("\\bVAR_");
     QString filename = "include/constants/vars.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &varNames);
+    varNames = parser.readCDefinesSorted(filename, prefixes);
     if (varNames.isEmpty()) {
         logError(QString("Failed to read var constants from %1").arg(filename));
         return false;
@@ -2157,11 +2154,10 @@ bool Project::readVarNames() {
 }
 
 bool Project::readMovementTypes() {
-    movementTypes.clear();
     QStringList prefixes("\\bMOVEMENT_TYPE_");
     QString filename = "include/constants/event_object_movement.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &movementTypes);
+    movementTypes = parser.readCDefinesSorted(filename, prefixes);
     if (movementTypes.isEmpty()) {
         logError(QString("Failed to read movement type constants from %1").arg(filename));
         return false;
@@ -2181,11 +2177,10 @@ bool Project::readInitialFacingDirections() {
 }
 
 bool Project::readMapTypes() {
-    mapTypes.clear();
     QStringList prefixes("\\bMAP_TYPE_");
     QString filename = "include/constants/map_types.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &mapTypes);
+    mapTypes = parser.readCDefinesSorted(filename, prefixes);
     if (mapTypes.isEmpty()) {
         logError(QString("Failed to read map type constants from %1").arg(filename));
         return false;
@@ -2194,11 +2189,10 @@ bool Project::readMapTypes() {
 }
 
 bool Project::readMapBattleScenes() {
-    mapBattleScenes.clear();
     QStringList prefixes("\\bMAP_BATTLE_SCENE_");
     QString filename = "include/constants/map_types.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted("include/constants/map_types.h", prefixes, &mapBattleScenes);
+    mapBattleScenes = parser.readCDefinesSorted("include/constants/map_types.h", prefixes);
     if (mapBattleScenes.isEmpty()) {
         logError(QString("Failed to read map battle scene constants from %1").arg(filename));
         return false;
@@ -2207,11 +2201,10 @@ bool Project::readMapBattleScenes() {
 }
 
 bool Project::readWeatherNames() {
-    weatherNames.clear();
     QStringList prefixes("\\bWEATHER_");
     QString filename = "include/constants/weather.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &weatherNames);
+    weatherNames = parser.readCDefinesSorted(filename, prefixes);
     if (weatherNames.isEmpty()) {
         logError(QString("Failed to read weather constants from %1").arg(filename));
         return false;
@@ -2220,13 +2213,13 @@ bool Project::readWeatherNames() {
 }
 
 bool Project::readCoordEventWeatherNames() {
-    if (!projectConfig.getEventWeatherTriggerEnabled()) return true;
+    if (!projectConfig.getEventWeatherTriggerEnabled())
+        return true;
 
-    coordEventWeatherNames.clear();
     QStringList prefixes("\\bCOORD_EVENT_WEATHER_");
     QString filename = "include/constants/weather.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &coordEventWeatherNames);
+    coordEventWeatherNames = parser.readCDefinesSorted(filename, prefixes);
     if (coordEventWeatherNames.isEmpty()) {
         logError(QString("Failed to read coord event weather constants from %1").arg(filename));
         return false;
@@ -2235,13 +2228,13 @@ bool Project::readCoordEventWeatherNames() {
 }
 
 bool Project::readSecretBaseIds() {
-    if (!projectConfig.getEventSecretBaseEnabled()) return true;
+    if (!projectConfig.getEventSecretBaseEnabled())
+        return true;
 
-    secretBaseIds.clear();
     QStringList prefixes("\\bSECRET_BASE_[A-Za-z0-9_]*_[0-9]+");
     QString filename = "include/constants/secret_bases.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &secretBaseIds);
+    secretBaseIds = parser.readCDefinesSorted(filename, prefixes);
     if (secretBaseIds.isEmpty()) {
         logError(QString("Failed to read secret base id constants from %1").arg(filename));
         return false;
@@ -2250,11 +2243,10 @@ bool Project::readSecretBaseIds() {
 }
 
 bool Project::readBgEventFacingDirections() {
-    bgEventFacingDirections.clear();
     QStringList prefixes("\\bBG_EVENT_PLAYER_FACING_");
     QString filename = "include/constants/event_bg.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &bgEventFacingDirections);
+    bgEventFacingDirections = parser.readCDefinesSorted(filename, prefixes);
     if (bgEventFacingDirections.isEmpty()) {
         logError(QString("Failed to read bg event facing direction constants from %1").arg(filename));
         return false;
@@ -2263,11 +2255,10 @@ bool Project::readBgEventFacingDirections() {
 }
 
 bool Project::readTrainerTypes() {
-    trainerTypes.clear();
     QStringList prefixes("\\bTRAINER_TYPE_");
     QString filename = "include/constants/trainer_types.h";
     fileWatcher.addPath(root + "/" + filename);
-    parser.readCDefinesSorted(filename, prefixes, &trainerTypes);
+    trainerTypes = parser.readCDefinesSorted(filename, prefixes);
     if (trainerTypes.isEmpty()) {
         logError(QString("Failed to read trainer type constants from %1").arg(filename));
         return false;