Skip to content

Commit

Permalink
Googletests and fixes for 'preprocessor.cpp'
Browse files Browse the repository at this point in the history
- Include 'string.h' instead of 'string'
- Delete external 'currentline' declaration, is already in 'cc_common''
- Introduce static buffer for script name for use in 'cc_error()'
- Set 'currentline' and 'ccCurScriptName' before 'cc_error()' is called, and _only_ there
- Use 'String::FromFormat()' for formatting the error messages

Googletests:
- Add external reference to 'ccCurScriptName'
- Add googletests for multiple scripts in one file; unterminated strings
- Add conversion to shut up 'signed/unsigned' warning
  • Loading branch information
fernewelten committed Dec 17, 2024
1 parent 7469daf commit 3eff26a
Show file tree
Hide file tree
Showing 5 changed files with 96 additions and 14 deletions.
18 changes: 11 additions & 7 deletions Compiler/preproc/preprocessor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
//=============================================================================
#include <algorithm>
#include <cctype>
#include <string>
#include <string.h>
#include "script/cs_parser_common.h"
#include "preproc/preprocessor.h"
#include "script/cc_common.h"
Expand All @@ -25,7 +25,6 @@

using namespace AGS::Common;

extern int currentline; // in script/script_common

namespace AGS {
namespace Preprocessor {
Expand All @@ -39,6 +38,7 @@ namespace Preprocessor {
#endif

const Error Preprocessor::NoError = Error();
String Preprocessor::_scriptOfError = {};

size_t FindIndexOfMatchingCharacter(String text, size_t indexOfFirstSpeechMark, int charToMatch)
{
Expand Down Expand Up @@ -149,6 +149,11 @@ namespace Preprocessor {
err.Message = msg;
_errors.push_back(err);

// 'cc_error()' will only work properly when the global variables
// 'currentline' and 'ccCurScriptName' are current
currentline = _lineNumber;
_scriptOfError = _scriptName;
ccCurScriptName = _scriptOfError.GetCStr();
cc_error(msg.GetCStr());
}

Expand Down Expand Up @@ -246,9 +251,8 @@ namespace Preprocessor {
size_t endOfString = FindIndexOfMatchingCharacter(text, i, text[i]);
if (endOfString == NOT_FOUND) //size_t is unsigned but it's alright
{
std::string msg = "Unterminated string: <char> is missing";
msg = msg.replace(msg.find("<char>"), 6, std::string(1, text[i]));
LogError(ErrorCode::UnterminatedString, msg.c_str());
String msg = String::FromFormat("Unterminated string: '%c' is missing", text[i]);
LogError(ErrorCode::UnterminatedString, msg);
break;
}
endOfString++;
Expand Down Expand Up @@ -478,15 +482,15 @@ namespace Preprocessor {
{
_errors.clear();
StringBuilder output = StringBuilder(script.GetLength());
currentline = _lineNumber = 0;
_lineNumber = 0;
String escapedScriptName = scriptName;
escapedScriptName.Replace("\\", "\\\\");
output.WriteLine(String::FromFormat("%s%s\"", NEW_SCRIPT_TOKEN_PREFIX, escapedScriptName.GetCStr()));
StringReader reader = StringReader(script);
_scriptName = scriptName;
while (reader.IsValid())
{
currentline = ++_lineNumber;
++_lineNumber;
String thisLine = reader.ReadLine();
thisLine = RemoveComments(thisLine);
if (thisLine.GetLength() > 0)
Expand Down
11 changes: 10 additions & 1 deletion Compiler/preproc/preprocessor.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,15 @@ namespace Preprocessor {
int _lineNumber;
String _scriptName;
Version _applicationVersion;
// A 'static' buffer for the script name when an error is
// reported with 'cc_error()'. We can't use the non-'static'
// field '_scriptName' for this purpose because the
// 'cc_error()' mechanism will need the buffer after this
// 'Preprocessor' object has been destroyed.
// TODO: Do away with '_scriptOfError' when 'Preprocessor'
// is refactored so that it doesn't use 'cc_error()' for
// error reporting
static String _scriptOfError;
// Conditional statement stack remembers the results of all the nested conditions
// that we have entered.
std::stack<bool> _conditionalStatements;
Expand Down Expand Up @@ -82,4 +91,4 @@ namespace Preprocessor {
};

} // Preprocessor
} // AGS
} // AGS
2 changes: 0 additions & 2 deletions Compiler/test/cc_test_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
#include "util/string_compat.h"
#include "util/string.h"

extern int currentline; // in script/script_common

typedef AGS::Common::String AGSString;

std::string last_cc_error_buf;
Expand Down
5 changes: 3 additions & 2 deletions Compiler/test/cc_test_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,6 @@ extern void clear_error(void);
extern const char *last_seen_cc_error(void);
extern std::pair<AGS::Common::String, AGS::Common::String> cc_error_at_line(const char* error_msg);
extern AGS::Common::String cc_error_without_line(const char* error_msg);

#endif // __CC_TEST_HELPER_H
extern int currentline;
extern const char *ccCurScriptName;
#endif // __CC_TEST_HELPER_H
74 changes: 72 additions & 2 deletions Compiler/test/preprocessor_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ namespace Preprocessor {
std::vector<AGSString> SplitLines(const AGSString& str)
{
std::vector<AGSString> str_lines = str.Split('\n');
for (int i = 0; i < str_lines.size(); i++) {
for (int i = 0; i < (int) str_lines.size(); i++) {
if (str_lines[i].CompareRight("\r", 1) == 0) {
str_lines[i].ClipRight(1);
}
Expand Down Expand Up @@ -812,5 +812,75 @@ void FuncИह€한𐍈() {
ASSERT_EQ(pp.GetLastError().Type, ErrorCode::InvalidCharacter);
}

TEST(Preprocess, UnterminatedStringSingleQuote) {
Preprocessor pp = Preprocessor();
const char *inpl = R"EOS(
int Func1()
{
'unterminated string
return 0;
}
)EOS";

clear_error();
String res = pp.Preprocess(inpl, "UnterminatedStringSingleQuote");

// According to the googletest docs,
// the expected value should come first, the tested expression second
// Then in the case of a failed test, the reported message comes out correctly.
EXPECT_STREQ("Unterminated string: ''' is missing", last_seen_cc_error());
auto err = pp.GetLastError();
// script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc
EXPECT_EQ(4, currentline);
}

TEST(Preprocess, UnterminatedStringDoubleQuote) {
Preprocessor pp = Preprocessor();
const char *inpl = R"EOS(
int Func1()
{
"unterminated string
return 0;
}
)EOS";

clear_error();
String res = pp.Preprocess(inpl, "UnterminatedStringDoubleQuote");

EXPECT_STREQ(last_seen_cc_error(), "Unterminated string: '\"' is missing");
// script lines are 1-based, because line 0 is a NEW SCRIPT MARKER added by preproc
EXPECT_EQ(currentline, 4);
}

TEST(Preprocess, MultipleNewScriptMarkers) {
Preprocessor pp = Preprocessor();
const char *inpl = R"EOS(
"__NEWSCRIPTSTART_Dialog1"
int Func1()
{
return 0;
}
"__NEWSCRIPTSTART_Dialog2"
int Func2()
{
"unterminated string
return 0;
}
"__NEWSCRIPTSTART_Dialog3"
int Func3()
{
return 0;
}
)EOS";

clear_error();
String res = pp.Preprocess(inpl, "MultipleNewScriptMarkers");

EXPECT_STREQ("Unterminated string: '\"' is missing", last_seen_cc_error());
EXPECT_STREQ("Dialog2", ccCurScriptName);
// Line count starts from the nearest NEW SCRIPT MARKER
EXPECT_EQ(3, currentline);
}

} // Preprocessor
} // AGS
} // AGS

0 comments on commit 3eff26a

Please sign in to comment.