From c2f805b2270a4eeef1f0560bfd38cac193d4e1bf Mon Sep 17 00:00:00 2001 From: Hugo Musso Gualandi Date: Tue, 13 Jun 2023 11:33:52 -0300 Subject: [PATCH 1/4] Linter: enforce that we use return value of check_exp If we don't use the return value of these functions, it results in a nasty and hard to find bug. The linter should be able to offer some protection, despite not being the perfect tool for the job. --- run-lint | 8 ++++++++ src/pallene/typechecker.lua | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/run-lint b/run-lint index 0e627bfb..ce058e34 100755 --- a/run-lint +++ b/run-lint @@ -41,6 +41,14 @@ do fi done +if grep --line-number -E \ + "^[$space$tab]*self:(check_exp|check_var|check_initializer)" \ + src/pallene/typechecker.lua; +then + # Using grep to catch this logic error isn't ideal, but it's better than nothing. + error "You should not ignore the return value here" +fi + if [ "$ok" != yes ]; then exit 1 fi diff --git a/src/pallene/typechecker.lua b/src/pallene/typechecker.lua index c52728cc..954e1122 100644 --- a/src/pallene/typechecker.lua +++ b/src/pallene/typechecker.lua @@ -33,7 +33,7 @@ -- -- IMPORTANT: For these transformations to work you should always use the return value from the -- check_exp and check_var functions. For example, instead of just `check_exp(foo.exp)` you should --- always write `foo.exp = check_exp(foo.exp)`. +-- always write `foo.exp = check_exp(foo.exp)`. Our linter script enforces this. local typechecker = {} From 8ac5f372813a0b51e8d183030a37105b3545eeae Mon Sep 17 00:00:00 2001 From: Hugo Musso Gualandi Date: Tue, 13 Jun 2023 11:34:19 -0300 Subject: [PATCH 2/4] Linter: run other checks even if Luacheck fails --- run-lint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-lint b/run-lint index ce058e34..aeff5f60 100755 --- a/run-lint +++ b/run-lint @@ -14,7 +14,7 @@ error() { } echo "--- Lua Lint ---" -luacheck src/ spec/ examples/ "$@" || exit 1 +luacheck src/ spec/ examples/ "$@" || error "Luacheck found problems" echo echo "--- Other checks ---" From 20d4fa70396fe171a0199f26b64f85c9982a855f Mon Sep 17 00:00:00 2001 From: Hugo Musso Gualandi Date: Tue, 13 Jun 2023 13:24:37 -0300 Subject: [PATCH 3/4] Nothing much --- run-lint | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/run-lint b/run-lint index aeff5f60..5fc50442 100755 --- a/run-lint +++ b/run-lint @@ -2,12 +2,13 @@ space=' ' tab=' ' -ok=yes red='\033[1;31m' green='\033[1;32m' reset='\033[0m' +ok=yes + error() { printf "${red}ERROR${reset} %s\n" "$*" ok=no From cd71262bc1d9be85cf9dc5e3ebd7227ac0a40c6f Mon Sep 17 00:00:00 2001 From: Hugo Musso Gualandi Date: Tue, 13 Jun 2023 13:25:18 -0300 Subject: [PATCH 4/4] Detect more cases of tabs mixed with spaces Thankfully hasn't happened yet --- run-lint | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/run-lint b/run-lint index 5fc50442..1cb290ab 100755 --- a/run-lint +++ b/run-lint @@ -32,7 +32,7 @@ do error "File $file has a line that ends in whitespace" fi - if grep --line-number "^$tab" "$file"; then + if grep --line-number "^$space*$tab" "$file"; then # Standardize on spaces because mixing tabs and spaces is endless pain. error "File $file has tab-based indentation" fi