(7.2) cleanup Petscii converter errorhandling, add unit tests for error scenarios

compiler/src/prog8/compiler/target/ICompilationTarget.kt

@@ -4,9 +4,6 @@ import com.github.michaelbull.result.fold
 import prog8.ast.IMemSizer
 import prog8.ast.Program
 import prog8.ast.base.*
-import prog8.ast.expressions.IdentifierReference
-import prog8.ast.expressions.NumericLiteralValue
-import prog8.ast.statements.AssignTarget
 import prog8.compiler.CompilationOptions
 import prog8.compiler.IErrorReporter
 import prog8.compiler.IStringEncoding
@@ -15,7 +12,6 @@ import prog8.compiler.target.c64.C64MachineDefinition
 import prog8.compiler.target.cbm.Petscii
 import prog8.compiler.target.cpu6502.codegen.AsmGen
 import prog8.compiler.target.cx16.CX16MachineDefinition
-import java.io.CharConversionException
 import java.nio.file.Path
 
 
@@ -38,11 +34,7 @@ internal object C64Target: ICompilationTarget {
         )
     }
     override fun decodeString(bytes: List<Short>, altEncoding: Boolean) =
-        try {
         if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true)
-        } catch (x: CharConversionException) {
-            throw CharConversionException("can't decode string: ${x.message}")
-        }
 
     override fun memorySize(dt: DataType): Int {
         return when(dt) {
@@ -66,11 +58,7 @@ internal object Cx16Target: ICompilationTarget {
         )
     }
     override fun decodeString(bytes: List<Short>, altEncoding: Boolean) =
-        try {
         if (altEncoding) Petscii.decodeScreencode(bytes, true) else Petscii.decodePetscii(bytes, true)
-        } catch (x: CharConversionException) {
-            throw CharConversionException("can't decode string: ${x.message}")
-        }
 
     override fun memorySize(dt: DataType): Int {
         return when(dt) {

compiler/src/prog8/compiler/target/cbm/Petscii.kt

@@ -1098,12 +1098,9 @@ object Petscii {
     fun decodePetscii(petscii: Iterable<Short>, lowercase: Boolean = false): String {
         return petscii.map {
             val code = it.toInt()
-            try {
+            if(code<0 || code>=decodingPetsciiLowercase.size)
+                throw CharConversionException("petscii $code out of range 0..${decodingPetsciiLowercase.size-1}")
             if(lowercase) decodingPetsciiLowercase[code] else decodingPetsciiUppercase[code]
-            } catch(x: CharConversionException) {
-                // TODO this CharConversionException can never occur?? also clean up ICompilationTarget.decodeString?
-                if(lowercase) decodingPetsciiUppercase[code] else decodingPetsciiLowercase[code]
-            }
         }.joinToString("")
     }
 
@@ -1140,17 +1137,15 @@ object Petscii {
     fun decodeScreencode(screencode: Iterable<Short>, lowercase: Boolean = false): String {
         return screencode.map {
             val code = it.toInt()
-            try {
+            if(code<0 || code>=decodingScreencodeLowercase.size)
+                throw CharConversionException("screencode $code out of range 0..${decodingScreencodeLowercase.size-1}")
             if (lowercase) decodingScreencodeLowercase[code] else decodingScreencodeUppercase[code]
-            } catch (x: CharConversionException) {
-                // TODO this CharConversionException can never occur?? also clean up ICompilationTarget.decodeString?
-                if (lowercase) decodingScreencodeUppercase[code] else decodingScreencodeLowercase[code]
-            }
         }.joinToString("")
     }
 
     fun petscii2scr(petscii_code: Short, inverseVideo: Boolean): Result<Short, CharConversionException> {
         val code = when {
+            petscii_code < 0 -> return Err(CharConversionException("petscii code out of range"))
             petscii_code <= 0x1f -> petscii_code + 128
             petscii_code <= 0x3f -> petscii_code.toInt()
             petscii_code <= 0x5f -> petscii_code - 64
@@ -1168,6 +1163,7 @@ object Petscii {
 
     fun scr2petscii(screencode: Short): Result<Short, CharConversionException> {
         val petscii = when {
+            screencode < 0 -> return Err(CharConversionException("screencode out of range"))
             screencode <= 0x1f -> screencode + 64
             screencode <= 0x3f -> screencode.toInt()
             screencode <= 0x5d -> screencode +123

compiler/test/TestPetscii.kt

@@ -1,15 +1,13 @@
 package prog8tests
 
 import com.github.michaelbull.result.Ok
+import com.github.michaelbull.result.expectError
 import org.hamcrest.MatcherAssert.assertThat
 import org.hamcrest.Matchers.equalTo
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.TestInstance
-import prog8.ast.base.DataType
-import prog8.ast.base.Position
-import prog8.ast.expressions.NumericLiteralValue
-import prog8.ast.expressions.StringLiteralValue
 import prog8.compiler.target.cbm.Petscii
+import java.io.CharConversionException
 import kotlin.test.*
 
 
@@ -34,8 +32,6 @@ class TestPetscii {
         assertThat("expect lowercase error fallback", Petscii.encodePetscii("♥", true), equalTo(Ok(listOf<Short>(0xd3))))
 
         assertThat(Petscii.decodePetscii(listOf(72, 0xd7, 0x5c, 0xfa, 0x12), true), equalTo("hW£✓\uF11A"))
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodePetscii(listOf(-1), true) }
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodePetscii(listOf(256), true) }
     }
 
     @Test
@@ -48,8 +44,6 @@ class TestPetscii {
         assertThat("expecting fallback", Petscii.encodePetscii("✓"), equalTo(Ok(listOf<Short>(250))))
 
         assertThat(Petscii.decodePetscii(listOf(72, 0x5c, 0xd3, 0xff)), equalTo("H£♥π"))
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodePetscii(listOf(-1)) }
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodePetscii(listOf(256)) }
     }
 
     @Test
@@ -62,8 +56,6 @@ class TestPetscii {
         assertThat("expect fallback", Petscii.encodeScreencode("π", true), equalTo(Ok(listOf<Short>(94))))
 
         assertThat(Petscii.decodeScreencode(listOf(0x08, 0x57, 0x1c, 0x7a), true), equalTo("hW£✓"))
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodeScreencode(listOf(-1), true) }
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodeScreencode(listOf(256), true) }
     }
 
     @Test
@@ -77,7 +69,29 @@ class TestPetscii {
         assertThat("expecting fallback", Petscii.encodeScreencode("✓"), equalTo(Ok(listOf<Short>(122))))
 
         assertThat(Petscii.decodeScreencode(listOf(0x17, 0x1c, 0x53, 0x5e)), equalTo("W£♥π"))
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodeScreencode(listOf(-1)) }
+    }
-        assertFailsWith<ArrayIndexOutOfBoundsException> { Petscii.decodeScreencode(listOf(256)) }
+
+    @Test
+    fun testErrorCases() {
+        Petscii.encodePetscii("~", true).expectError { "shouldn't be able to encode tilde" }
+        Petscii.encodePetscii("~", false).expectError { "shouldn't be able to encode tilde" }
+        Petscii.encodeScreencode("~", true).expectError { "shouldn't be able to encode tilde" }
+        Petscii.encodeScreencode("~", false).expectError { "shouldn't be able to encode tilde" }
+
+        assertFailsWith<CharConversionException> { Petscii.decodePetscii(listOf<Short>(-1), true) }
+        assertFailsWith<CharConversionException> { Petscii.decodePetscii(listOf<Short>(256), true) }
+        assertFailsWith<CharConversionException> { Petscii.decodePetscii(listOf<Short>(-1), false) }
+        assertFailsWith<CharConversionException> { Petscii.decodePetscii(listOf<Short>(256), false) }
+        assertFailsWith<CharConversionException> { Petscii.decodeScreencode(listOf<Short>(-1), true) }
+        assertFailsWith<CharConversionException> { Petscii.decodeScreencode(listOf<Short>(256), true) }
+        assertFailsWith<CharConversionException> { Petscii.decodeScreencode(listOf<Short>(-1), false) }
+        assertFailsWith<CharConversionException> { Petscii.decodeScreencode(listOf<Short>(256), false) }
+
+        Petscii.scr2petscii(-1).expectError { "-1 should error" }
+        Petscii.scr2petscii(256).expectError { "256 should error" }
+        Petscii.petscii2scr(-1, true).expectError { "-1 should error" }
+        Petscii.petscii2scr(256, true).expectError { "256 should error" }
+        Petscii.petscii2scr(-1, false).expectError { "-1 should error" }
+        Petscii.petscii2scr(256, false).expectError { "256 should error" }
     }
 }

compilerAst/src/prog8/ast/expressions/AstExpressions.kt

@@ -513,7 +513,7 @@ class CharLiteral(val value: Char,
     }
 
     override fun referencesIdentifier(vararg scopedName: String) = false
-    override fun constValue(program: Program): NumericLiteralValue? = null  // TODO: CharLiteral.constValue can't be NumericLiteralValue...
+    override fun constValue(program: Program): NumericLiteralValue? = null  // TODO: CharLiteral.constValue can't be NumericLiteralValue... unless we re-add string encoder to program?
     override fun accept(visitor: IAstVisitor) = visitor.visit(this)
     override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent)