Skip to content

Commit

Permalink
One read() method for parser and linter. Tests use multiline string f…
Browse files Browse the repository at this point in the history
…or code. (#20)
  • Loading branch information
disinvite authored Nov 21, 2024
1 parent 9ca6e5f commit 90739f1
Show file tree
Hide file tree
Showing 6 changed files with 407 additions and 399 deletions.
2 changes: 1 addition & 1 deletion reccmp/isledecomp/parser/codebase.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def __init__(self, filenames: Iterable[str], module: str) -> None:
for filename in filenames:
parser.reset()
with open(filename, "r", encoding="utf-8") as f:
parser.read_lines(f)
parser.read(f.read())

for sym in parser.iter_symbols(module):
sym.filename = filename
Expand Down
11 changes: 4 additions & 7 deletions reccmp/isledecomp/parser/linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,15 +115,12 @@ def _check_byname_allowed(self):
)
)

def check_lines(self, lines, filename, module=None):
"""`lines` is a generic iterable to allow for testing with a list of strings.
We assume lines has the entire contents of the compilation unit."""

def read(self, code: str, filename: str, module=None) -> bool:
self.reset(False)
self._filename = filename
self._module = module

self._parser.read_lines(lines)
self._parser.read(code)

self._parser.finish()
self.alerts = self._parser.alerts[::]
Expand All @@ -138,7 +135,7 @@ def check_lines(self, lines, filename, module=None):

return len(self.alerts) == 0

def check_file(self, filename, module=None):
def check_file(self, filename: str, module=None):
"""Convenience method for decomplint cli tool"""
with open(filename, "r", encoding="utf-8") as f:
return self.check_lines(f, filename, module)
return self.read(f.read(), filename, module)
7 changes: 4 additions & 3 deletions reccmp/isledecomp/parser/parser.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# C++ file parser

from typing import List, Iterable, Iterator, Optional
import io
from typing import List, Iterator, Optional
from enum import Enum
from .util import (
get_class_name,
Expand Down Expand Up @@ -545,8 +546,8 @@ def read_line(self, line: str):
if vtable_class is not None:
self._vtable_done(class_name=vtable_class)

def read_lines(self, lines: Iterable):
for line in lines:
def read(self, text: str):
for line in io.StringIO(text, newline=None):
self.read_line(line)

def finish(self):
Expand Down
144 changes: 72 additions & 72 deletions tests/test_linter.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,27 @@ def fixture_linter():


def test_simple_in_order(linter):
lines = [
"// FUNCTION: TEST 0x1000",
"void function1() {}",
"// FUNCTION: TEST 0x2000",
"void function2() {}",
"// FUNCTION: TEST 0x3000",
"void function3() {}",
]
assert linter.check_lines(lines, "test.cpp", "TEST") is True
code = """\
// FUNCTION: TEST 0x1000
void function1() {}
// FUNCTION: TEST 0x2000
void function2() {}
// FUNCTION: TEST 0x3000
void function3() {}
"""
assert linter.read(code, "test.cpp", "TEST") is True


def test_simple_not_in_order(linter):
lines = [
"// FUNCTION: TEST 0x1000",
"void function1() {}",
"// FUNCTION: TEST 0x3000",
"void function3() {}",
"// FUNCTION: TEST 0x2000",
"void function2() {}",
]
assert linter.check_lines(lines, "test.cpp", "TEST") is False
code = """\
// FUNCTION: TEST 0x1000
void function1() {}
// FUNCTION: TEST 0x3000
void function3() {}
// FUNCTION: TEST 0x2000
void function2() {}
"""
assert linter.read(code, "test.cpp", "TEST") is False
assert len(linter.alerts) == 1

assert linter.alerts[0].code == ParserError.FUNCTION_OUT_OF_ORDER
Expand All @@ -39,16 +39,16 @@ def test_simple_not_in_order(linter):

def test_byname_ignored(linter):
"""Should ignore lookup-by-name markers when checking order."""
lines = [
"// FUNCTION: TEST 0x1000",
"void function1() {}",
"// FUNCTION: TEST 0x3000",
"// MyClass::MyMethod",
"// FUNCTION: TEST 0x2000",
"void function2() {}",
]
code = """\
// FUNCTION: TEST 0x1000
void function1() {}
// FUNCTION: TEST 0x3000
// MyClass::MyMethod
// FUNCTION: TEST 0x2000
void function2() {}
"""
# This will fail because byname lookup does not belong in the cpp file
assert linter.check_lines(lines, "test.cpp", "TEST") is False
assert linter.read(code, "test.cpp", "TEST") is False
# but it should not fail for function order.
assert all(
alert.code != ParserError.FUNCTION_OUT_OF_ORDER for alert in linter.alerts
Expand All @@ -57,88 +57,88 @@ def test_byname_ignored(linter):

def test_module_isolation(linter):
"""Should check the order of markers from a single module only."""
lines = [
"// FUNCTION: ALPHA 0x0001",
"// FUNCTION: TEST 0x1000",
"void function1() {}",
"// FUNCTION: ALPHA 0x0002",
"// FUNCTION: TEST 0x2000",
"void function2() {}",
"// FUNCTION: ALPHA 0x0003",
"// FUNCTION: TEST 0x3000",
"void function3() {}",
]

assert linter.check_lines(lines, "test.cpp", "TEST") is True
code = """\
// FUNCTION: ALPHA 0x0001
// FUNCTION: TEST 0x1000
void function1() {}
// FUNCTION: ALPHA 0x0002
// FUNCTION: TEST 0x2000
void function2() {}
// FUNCTION: ALPHA 0x0003
// FUNCTION: TEST 0x3000
void function3() {}
"""

assert linter.read(code, "test.cpp", "TEST") is True
linter.reset(True)
assert linter.check_lines(lines, "test.cpp", "ALPHA") is True
assert linter.read(code, "test.cpp", "ALPHA") is True


def test_byname_headers_only(linter):
"""Markers that ar referenced by name with cvdump belong in header files only."""
lines = [
"// FUNCTION: TEST 0x1000",
"// MyClass::~MyClass",
]
code = """\
// FUNCTION: TEST 0x1000
// MyClass::~MyClass
"""

assert linter.check_lines(lines, "test.h", "TEST") is True
assert linter.read(code, "test.h", "TEST") is True
linter.reset(True)
assert linter.check_lines(lines, "test.cpp", "TEST") is False
assert linter.read(code, "test.cpp", "TEST") is False
assert linter.alerts[0].code == ParserError.BYNAME_FUNCTION_IN_CPP


def test_duplicate_offsets(linter):
"""The linter will retain module/offset pairs found until we do a full reset."""
lines = [
"// FUNCTION: TEST 0x1000",
"// FUNCTION: HELLO 0x1000",
"// MyClass::~MyClass",
]
code = """\
// FUNCTION: TEST 0x1000
// FUNCTION: HELLO 0x1000
// MyClass::~MyClass
"""

# Should not fail for duplicate offset 0x1000 because the modules are unique.
assert linter.check_lines(lines, "test.h", "TEST") is True
assert linter.read(code, "test.h", "TEST") is True

# Simulate a failure by reading the same file twice.
assert linter.check_lines(lines, "test.h", "TEST") is False
assert linter.read(code, "test.h", "TEST") is False

# Two errors because offsets from both modules are duplicated
assert len(linter.alerts) == 2
assert all(a.code == ParserError.DUPLICATE_OFFSET for a in linter.alerts)

# Partial reset will retain the list of seen offsets.
linter.reset(False)
assert linter.check_lines(lines, "test.h", "TEST") is False
assert linter.read(code, "test.h", "TEST") is False

# Full reset will forget seen offsets.
linter.reset(True)
assert linter.check_lines(lines, "test.h", "TEST") is True
assert linter.read(code, "test.h", "TEST") is True


def test_duplicate_strings(linter):
"""Duplicate string markers are okay if the string value is the same."""
string_lines = [
"// STRING: TEST 0x1000",
'return "hello world";',
]
string_lines = """\
// STRING: TEST 0x1000
return "hello world";
"""

# No problem to use this marker twice.
assert linter.check_lines(string_lines, "test.h", "TEST") is True
assert linter.check_lines(string_lines, "test.h", "TEST") is True
assert linter.read(string_lines, "test.h", "TEST") is True
assert linter.read(string_lines, "test.h", "TEST") is True

different_string = [
"// STRING: TEST 0x1000",
'return "hi there";',
]
different_string = """\
// STRING: TEST 0x1000
return "hi there";
"""

# Same address but the string is different
assert linter.check_lines(different_string, "greeting.h", "TEST") is False
assert linter.read(different_string, "greeting.h", "TEST") is False
assert len(linter.alerts) == 1
assert linter.alerts[0].code == ParserError.WRONG_STRING

same_addr_reused = [
"// GLOBAL:TEXT 0x1000",
"int g_test = 123;",
]
same_addr_reused = """\
// GLOBAL:TEXT 0x1000
int g_test = 123;
"""

# This will fail like any other offset reuse.
assert linter.check_lines(same_addr_reused, "other.h", "TEST") is False
assert linter.read(same_addr_reused, "other.h", "TEST") is False
Loading

0 comments on commit 90739f1

Please sign in to comment.