[education/rkward] rkward: Add seom first autotests, and fix the bugs, encountered
Thomas Friedrichsmeier
null at kde.org
Mon May 26 15:06:38 BST 2025
Git commit 776d84594602d8ee62f5e5d3e72f5656d662ae8c by Thomas Friedrichsmeier.
Committed on 26/05/2025 at 14:06.
Pushed by tfry into branch 'master'.
Add seom first autotests, and fix the bugs, encountered
M +3 -0 rkward/autotests/CMakeLists.txt
A +31 -0 rkward/autotests/data/script1.R
A +135 -0 rkward/autotests/rkparsedscript_test.cpp [License: GPL(v2.0+)]
M +57 -32 rkward/misc/rkparsedscript.cpp
M +7 -3 rkward/misc/rkparsedscript.h
https://invent.kde.org/education/rkward/-/commit/776d84594602d8ee62f5e5d3e72f5656d662ae8c
diff --git a/rkward/autotests/CMakeLists.txt b/rkward/autotests/CMakeLists.txt
index b53052f1a..8d100310d 100644
--- a/rkward/autotests/CMakeLists.txt
+++ b/rkward/autotests/CMakeLists.txt
@@ -17,6 +17,9 @@ macro(rkward_executable_tests)
endforeach()
endmacro()
+add_definitions(-DTEST_DATA_DIR=\"${CMAKE_CURRENT_SOURCE_DIR}//data/\")
+
rkward_executable_tests(
core_test
+ rkparsedscript_test
)
diff --git a/rkward/autotests/data/script1.R b/rkward/autotests/data/script1.R
new file mode 100644
index 000000000..9dd7f0a87
--- /dev/null
+++ b/rkward/autotests/data/script1.R
@@ -0,0 +1,31 @@
+# Comment 1
+# Comment 2
+Symbol00 + 1
+Symbol01 <- Symbol2(Symbol03="String1", function(Symbol04) {
+ for (Symbol05 in Symbol06) {
+ Symbol07
+ Symbol08$Symbol09[1:10]
+ Symbol10(Symbol11=Symbol12, Symbol13 + Symbol14)
+ Symbol15; Symbol16 <- Symbol17 + 1
+ Symbol18
+ }
+})
+Symbol19
+
+# Comment 4
+Symbol.x <- Symbol.y + Symbol.z +
+ Symbol.y
+
+FunctionList <- list(
+ Argname=function() { statement },
+ makeFunction({ aaa; bbb; ccc; },
+ {
+ nest1
+ {
+ nest2
+ nest3
+ nest4
+ }
+ nest5
+ }, ddd, eee <- fff(ggg + hhh) + iii, jjj)
+)
diff --git a/rkward/autotests/rkparsedscript_test.cpp b/rkward/autotests/rkparsedscript_test.cpp
new file mode 100644
index 000000000..cdf5d1aa9
--- /dev/null
+++ b/rkward/autotests/rkparsedscript_test.cpp
@@ -0,0 +1,135 @@
+/*
+core_test - This file is part of RKWard (https://rkward.kde.org). Created: Sun May 18 2025
+SPDX-FileCopyrightText: 2025 by Thomas Friedrichsmeier <thomas.friedrichsmeier at kdemail.net>
+SPDX-FileContributor: The RKWard Team <rkward-devel at kde.org>
+SPDX-License-Identifier: GPL-2.0-or-later
+*/
+
+#include <QFile>
+#include <QStandardPaths>
+#include <QTest>
+
+#include "../misc/rkparsedscript.h"
+
+using namespace Qt::Literals::StringLiterals;
+
+void testLog(const char *fmt, va_list args) {
+ vprintf(fmt, args);
+ printf("\n");
+ fflush(stdout);
+}
+
+void testLog(const char *fmt, ...) {
+ va_list ap;
+ va_start(ap, fmt);
+ testLog(fmt, ap);
+ va_end(ap);
+}
+
+void RKDebug(int, int level, const char *fmt, ...) {
+ va_list ap;
+ va_start(ap, fmt);
+ testLog(fmt, ap);
+ if (level >= DL_ERROR) QFAIL("error message during test (see above)");
+ va_end(ap);
+}
+
+/** Test RKParsedScript class. */
+class RKParsedScriptTest : public QObject {
+ Q_OBJECT
+ private:
+ QString script;
+ RKParsedScript ps;
+
+ void loadScript(const QString &relname) {
+ QFile f(QStringLiteral(TEST_DATA_DIR) + relname);
+ bool ok = f.open(QIODevice::ReadOnly | QIODevice::Text);
+ QVERIFY(ok);
+ script = QString::fromUtf8(f.readAll());
+ ps = RKParsedScript(script);
+ }
+
+ void compareScript(RKParsedScript::ContextIndex pos, const QString &expected) {
+ auto ctx = ps.getContext(pos);
+ QCOMPARE(script.mid(ctx.start, expected.length()), expected);
+ }
+
+ RKParsedScript::ContextIndex moveAndCheck(const RKParsedScript::ContextIndex newpos, const QString &expected) {
+ compareScript(newpos, expected);
+ return newpos;
+ }
+ private Q_SLOTS:
+ void init() {
+ testLog("Starting next test");
+ }
+
+ void cleanup() {
+ testLog("Cleanup");
+ }
+
+ void initTestCase() {
+ QStandardPaths::setTestModeEnabled(true);
+ RK_Debug::RK_Debug_Level = DL_DEBUG;
+ }
+
+ void nextPrevStatement() {
+ loadScript(u"script1.R"_s);
+// ps.serialize();
+ auto ctx = ps.contextAtPos(0);
+ QVERIFY(ctx.valid());
+ testLog("Block1");
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol00"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol01"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol19"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"Symbol.x"_s);
+ ctx = moveAndCheck(ps.nextStatement(ctx), u"FunctionList"_s);
+ QVERIFY(!ps.nextStatement(ctx).valid()); // NOTE this need not be set in stone
+
+ testLog("Block2");
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol.x"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol19"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol01"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol00"_s);
+ //QVERIFY(!ps.prevStatement(ctx).valid()); // not sure that we want this
+
+ testLog("Block3");
+ const auto symb18 = ps.contextAtPos(script.indexOf(u"Symbol18"));
+ ctx = moveAndCheck(ps.prevStatement(symb18), u"Symbol16"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol15"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol10"_s); // shall skip inner
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol08"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol07"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"for"_s); // shall progressively move out, from here
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"function"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol03"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol01"_s);
+
+ testLog("Block4");
+ const auto symb14 = ps.contextAtPos(script.indexOf(u"Symbol14"));
+ ctx = moveAndCheck(ps.prevStatement(symb14), u"Symbol11"_s); // shall stay in parenthesis
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Symbol10"_s); // shall move out
+
+ testLog("Block5");
+ const auto symbjjj = ps.contextAtPos(script.indexOf(u"jjj"));
+ ctx = moveAndCheck(ps.prevStatement(symbjjj), u"eee"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"ddd"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"{"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"{ aaa"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"makeFunction"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"Argname"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"FunctionList"_s);
+
+ testLog("Block6");
+ const auto symbnest3 = ps.contextAtPos(script.indexOf(u"nest3"));
+ ctx = moveAndCheck(ps.prevStatement(symbnest3), u"nest2"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"{"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"nest1"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"{"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"{ aaa"_s);
+ ctx = moveAndCheck(ps.prevStatement(ctx), u"makeFunction"_s);
+ }
+};
+
+QTEST_MAIN(RKParsedScriptTest)
+
+#include "rkparsedscript_test.moc"
diff --git a/rkward/misc/rkparsedscript.cpp b/rkward/misc/rkparsedscript.cpp
index 307db4382..1ad78a93e 100644
--- a/rkward/misc/rkparsedscript.cpp
+++ b/rkward/misc/rkparsedscript.cpp
@@ -13,12 +13,15 @@ SPDX-License-Identifier: GPL-2.0-or-later
#include "../debug.h"
RKParsedScript::RKParsedScript(const QString &content) : prevtype(None), allow_merge(true) {
- context_list.reserve(200); // just a very wild guess
+ RK_TRACE(MISC);
+
+ context_list.reserve(200); // just a very wild guess.
addContext(Top, -1, content);
- RK_ASSERT(context_list.front().end == content.size() -1);
+ RK_ASSERT(context_list.front().end == content.size());
};
int RKParsedScript::addContext(ContextType type, int start, const QString &content) {
+ RK_TRACE(MISC);
int index = context_list.size();
// some contexts need (or benefit from) special handling depending on the preceding context
// TODO: maybe it would be easier to do this in a separte post-processing step
@@ -91,12 +94,18 @@ int RKParsedScript::addContext(ContextType type, int start, const QString &conte
// NOTE: we can't just keep a reference to the context at the start of this function, as the vector
// may re-allocate during nested parsing
- context_list.at(index).end = pos;
+ if (type == Comment) {
+ // following delimiter shall not be considered part of the comment
+ context_list.at(index).end = pos - 1;
+ } else {
+ context_list.at(index).end = pos;
+ }
prevtype = type;
return pos;
};
RKParsedScript::ContextIndex RKParsedScript::contextAtPos(int pos) const {
+ RK_TRACE(MISC);
// Context 0 is Top, not really of interest
for (unsigned int i = 1; i < context_list.size(); ++i) {
if (context_list.at(i).start > pos) {
@@ -120,6 +129,7 @@ RKParsedScript::ContextIndex RKParsedScript::prevContext(const ContextIndex from
/** Find the innermost region (Context::maybeNesting()) containing this context */
RKParsedScript::ContextIndex RKParsedScript::parentRegion(const ContextIndex from) const {
+ RK_TRACE(MISC);
if (from.valid()) {
int startpos = context_list.at(from.index).start;
for (int i = from.index-1; i >= 0; --i) {
@@ -133,6 +143,7 @@ RKParsedScript::ContextIndex RKParsedScript::parentRegion(const ContextIndex fro
/** Find next sibling context (no inner or outer contexts) */
RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(const ContextIndex from) const {
+ RK_TRACE(MISC);
if (from.valid()) {
int endpos = context_list.at(from.index).end;
unsigned int i = from.index;
@@ -145,26 +156,27 @@ RKParsedScript::ContextIndex RKParsedScript::nextSiblingOrOuter(const ContextInd
}
RKParsedScript::ContextIndex RKParsedScript::nextSibling(const ContextIndex from) const {
+ RK_TRACE(MISC);
auto candidate = nextSiblingOrOuter(from);
if (parentRegion(candidate) == parentRegion(from)) return candidate;
return ContextIndex();
}
RKParsedScript::ContextIndex RKParsedScript::prevSiblingOrOuter(const ContextIndex from) const {
+ RK_TRACE(MISC);
if (from.valid()) {
int startpos = context_list.at(from.index).start;
+ const auto parent = parentRegion(from);
int i = from.index;
- do {
- --i;
- } while (i >= 0 && context_list.at(i).end >= startpos); // advance, skipping child regions
-
while (true) {
- // handle the case of "a(b)c": Moving rev from c would give us b, however, that's a nephew, not a sibling
- // In this situation we want to return b's parent (i.e. the opening brace)
- // In a loop, as the situation could also be "a{(b)}c", etc.
- auto candidate_parent = parentRegion(ContextIndex(i));
- if (getContext(candidate_parent).end >= startpos) break;
- i = candidate_parent.index;
+ --i;
+ if (i < 0) break;
+ const auto cparent = parentRegion(ContextIndex(i));
+ // sibling must have the same parent as us (consider reversing from "c" in "a(b)c"; we shall not stop at b)
+ if (cparent == parent) break;
+ // but we also allow outer contexts, i.e. parent of previous is a grandparent of us
+ if (!cparent.valid()) break;
+ if (context_list.at(cparent.index).end >= startpos) break;
}
return ContextIndex(i);
}
@@ -172,15 +184,33 @@ RKParsedScript::ContextIndex RKParsedScript::prevSiblingOrOuter(const ContextInd
}
RKParsedScript::ContextIndex RKParsedScript::prevSibling(const ContextIndex from) const {
+ RK_TRACE(MISC);
auto candidate = prevSiblingOrOuter(from);
if (parentRegion(candidate) == parentRegion(from)) return candidate;
return ContextIndex();
}
-RKParsedScript::ContextIndex RKParsedScript::nextStatement(const ContextIndex from) const {
+RKParsedScript::ContextIndex RKParsedScript::firstContextInStatement(const ContextIndex from) const {
+ RK_TRACE(MISC);
+
+ auto pi = from;
+ while (true) {
+ pi = prevSiblingOrOuter(pi);
+ if (!pi.valid()) break; // hit top end
+ const auto &pc = getContext(pi);
+ if (pc.type == Delimiter) break;
+ // Consider reversing from "c" in "a + (b + c)" bs. "a + (b) + c": We want to stop at region
+ // openings, too, *if* that region is a parent of this one
+ if (pc.maybeNesting() && pc.end > getContext(from).start) break;
+ }
+ return nextContext(pi);
+}
+
+RKParsedScript::ContextIndex RKParsedScript::lastContextInStatement(const ContextIndex from) const {
+ RK_TRACE(MISC);
+
auto ni = from;
auto cparent = parentRegion(from);
-
// skip over anything that is not an explicit or implicit delimiter
while (true) {
ni = nextSiblingOrOuter(ni);
@@ -188,7 +218,14 @@ RKParsedScript::ContextIndex RKParsedScript::nextStatement(const ContextIndex fr
else if (getContext(ni).type == Delimiter) break;
else if (getContext(ni).start > getContext(cparent).end) break;
}
+ return prevContext(ni);
+}
+
+RKParsedScript::ContextIndex RKParsedScript::nextStatement(const ContextIndex from) const {
+ RK_TRACE(MISC);
+ // forward past end of current statement
+ auto ni = nextContext(lastContextInStatement(from));
// skip over any following non-interesting contexts
while (true) {
auto type = getContext(ni).type;
@@ -199,30 +236,18 @@ RKParsedScript::ContextIndex RKParsedScript::nextStatement(const ContextIndex fr
}
RKParsedScript::ContextIndex RKParsedScript::prevStatement(const ContextIndex from) const {
- auto pi = from;
+ RK_TRACE(MISC);
- // TODO: refactor into more readable / reusable parts: statementStart() and statementEnd()
-
- // find start of current statement (the delimiter, preceding it)
- while (true) {
- pi = prevSiblingOrOuter(pi);
- if (!pi.valid()) break; // hit top end
- else if (getContext(pi).type == Delimiter) break;
- }
- // from here, skip over any preceding non-interesting contexts (entering the previous statement
+ // start at the first context preceeding the current statement
+ auto pi = prevContext(firstContextInStatement(from));
+ // from here, skip over any preceding non-interesting contexts (entering the previous statement)
while (true) {
auto type = getContext(pi).type;
if (type != Delimiter && type != Comment) break;
pi = prevSiblingOrOuter(pi);
}
// now find the start of the previous statement
- while (true) {
- pi = prevSiblingOrOuter(pi);
- if (!pi.valid()) break; // hit top end
- else if (getContext(pi).type == Delimiter) break;
- }
- // actual start is the token after the delimiter
- return nextContext(pi);
+ return (firstContextInStatement(pi));
}
// NOTE: used in debugging, only
diff --git a/rkward/misc/rkparsedscript.h b/rkward/misc/rkparsedscript.h
index 95b156573..b5db0601c 100644
--- a/rkward/misc/rkparsedscript.h
+++ b/rkward/misc/rkparsedscript.h
@@ -51,7 +51,7 @@ class RKParsedScript {
int end;
};
- RKParsedScript(const QString &content);
+ explicit RKParsedScript(const QString &content=QString());
enum SearchFlags {
NoFlags,
@@ -64,8 +64,8 @@ class RKParsedScript {
ContextIndex() : index(-1) {};
explicit ContextIndex(int index) : index(index) {};
bool valid() const { return index >= 0; };
- bool operator==(const ContextIndex &other) { return index == other.index; };
- bool operator!=(const ContextIndex &other) { return index != other.index; };
+ bool operator==(const ContextIndex &other) const { return index == other.index; };
+ bool operator!=(const ContextIndex &other) const { return index != other.index; };
int index;
};
@@ -88,6 +88,9 @@ class RKParsedScript {
ContextIndex nextStatement(const ContextIndex from) const;
ContextIndex prevStatement(const ContextIndex from) const;
+ ContextIndex firstContextInStatement(const ContextIndex from) const;
+ ContextIndex lastContextInStatement(const ContextIndex from) const;
+
/** retrieve the context at the given index. Safe to call, even with an invalid index
* (in which case the outermost context will be returned). */
const Context &getContext(const ContextIndex index) const {
@@ -99,6 +102,7 @@ class RKParsedScript {
// add and parse a context. This is where the actual parsing takes place
int addContext(ContextType type, int start, const QString &content);
+friend class RKParsedScriptTest;
friend class RKCodeNavigation;
// NOTE: used in debugging, only
QString serialize() const;
More information about the rkward-tracker
mailing list