From 96ba81e24aef2eadfd663fcea6adcea502c19a7b Mon Sep 17 00:00:00 2001 From: Gleb Koval Date: Sun, 9 Mar 2025 23:37:04 +0000 Subject: [PATCH] refactor: consistent error handling in Main.scala --- project.scala | 2 - src/main/wacc/Main.scala | 126 +++++++++++------------------ src/main/wacc/frontend/Error.scala | 23 +++++- src/main/wacc/frontend/ast.scala | 1 - 4 files changed, 69 insertions(+), 83 deletions(-) diff --git a/project.scala b/project.scala index d8b5880..8edf035 100644 --- a/project.scala +++ b/project.scala @@ -20,5 +20,3 @@ //> using options -Yexplicit-nulls -Wsafe-init -Xkind-projector:underscores // repositories for pre-release versions if needed -//> using repositories sonatype-s01:releases -//> using repositories sonatype-s01:snapshots diff --git a/src/main/wacc/Main.scala b/src/main/wacc/Main.scala index 33c4fef..d964657 100644 --- a/src/main/wacc/Main.scala +++ b/src/main/wacc/Main.scala @@ -12,7 +12,6 @@ import cats.effect.ExitCode import com.monovore.decline._ import com.monovore.decline.effect._ -import com.monovore.decline.Argument import org.typelevel.log4cats.slf4j.Slf4jLogger import org.typelevel.log4cats.Logger @@ -29,6 +28,9 @@ TODO: 5) general cleanup and comments (things like replacing home/ with ~ , and names of parameters and args, descriptions etc) */ +private val SUCCESS = ExitCode.Success.code +private val ERROR = ExitCode.Error.code + given logger: Logger[IO] = Slf4jLogger.getLogger[IO] val logOpt: Opts[Boolean] = @@ -51,49 +53,46 @@ val filesOpt: Opts[NonEmptyList[Path]] = val outputOpt: Opts[Option[Path]] = Opts .option[Path]("output", metavar = "path", help = "Output directory for compiled files.") + .validate("Must have permissions to create & access the output path") { path => + try { + Files.createDirectories(path) + true + } catch { + case e: java.nio.file.AccessDeniedException => + false + } + } + .validate("Output path must be a directory") { path => + Files.isDirectory(path) + } .orNone -val greedyOpt: Opts[Boolean] = - Opts.flag("greedy", "Compile WACC files sequentially instead of parallelly", short = "g").orFalse - def frontend( contents: String -): IO[Either[Int, microWacc.Program]] = { - IO(parser.parse(contents)).flatMap { - case Failure(msg) => - logger.error(s"Syntax error: $msg").as(Left(100)) - +): Either[NonEmptyList[Error], microWacc.Program] = + parser.parse(contents) match { + case Failure(msg) => Left(NonEmptyList.one(Error.SyntaxError(msg))) case Success(prog) => given errors: mutable.Builder[Error, List[Error]] = List.newBuilder val (names, funcs) = renamer.rename(prog) given ctx: typeChecker.TypeCheckerCtx = typeChecker.TypeCheckerCtx(names, funcs, errors) - val typedProg = typeChecker.check(prog) - val errResult = errors.result - - if (errResult.isEmpty) IO.pure(Right(typedProg)) - else { - // TODO: multiple traversal of error content, should be a foldleft or co - given errorContent: String = contents - val exitCode = errResult - .collectFirst { case _: Error.InternalError => - 201 - } - .getOrElse(200) - - val formattedErrors = errResult.map(formatError).mkString("\n") - - logger.error(s"Semantic errors:\n$formattedErrors").as(Left(exitCode)) + NonEmptyList.fromList(errors.result) match { + case Some(errors) => Left(errors) + case None => Right(typedProg) } } -} def backend(typedProg: microWacc.Program): Chain[asm.AsmLine] = asmGenerator.generateAsm(typedProg) -def compile(filePath: Path, outputDir: Option[Path], log: Boolean): IO[Int] = { +def compile( + filePath: Path, + outputDir: Option[Path], + log: Boolean +): IO[Int] = { val logAction: String => IO[Unit] = if (log) logger.info(_) else (_ => IO.unit) @@ -101,79 +100,49 @@ def compile(filePath: Path, outputDir: Option[Path], log: Boolean): IO[Int] = { def readSourceFile: IO[String] = IO.blocking(os.read(os.Path(filePath))) - def ensureOutputDir(outDir: Path): IO[Path] = - IO.blocking { - Files.createDirectories(outDir) - outDir - }.handleErrorWith { - // TODO: I think this wont occur if a user runs with privileges but this must be checked - // TODO: this will return the ugly stack trace, one could refactor compileCommand to - case _: java.nio.file.AccessDeniedException => - IO.raiseError( - new Exception( - s"Permission denied: Cannot create directory '${outDir.toAbsolutePath}'. Try choosing a different output path or run as root." - ) - ) - } - // TODO: path, file , the names are confusing (when Path is the type but we are working with files) def writeOutputFile(typedProg: microWacc.Program, outputPath: Path): IO[Unit] = writer.writeTo(backend(typedProg), outputPath) *> logger.info(s"Success: ${outputPath.toAbsolutePath}") def processProgram(contents: String, outDir: Path): IO[Int] = - frontend(contents).flatMap { - case Left(code) => - logger.error(s"Compilation failed for $filePath\nExit code: $code").as(code) + frontend(contents) match { + case Left(errors) => + val code = errors.map(err => err.exitCode).toList.min + given errorContent: String = contents + val errorMsg = errors.map(formatError).toIterable.mkString("\n") + for { + _ <- logAction(s"Compilation failed for $filePath\nExit code: $code") + _ <- IO.blocking( + // Explicit println since we want this to always show without logger thread info e.t.c. + println(s"Compilation failed for ${filePath.toAbsolutePath}:\n$errorMsg") + ) + } yield code case Right(typedProg) => val outputFile = outDir.resolve(filePath.getFileName.toString.stripSuffix(".wacc") + ".s") - writeOutputFile(typedProg, outputFile).as(0) + writeOutputFile(typedProg, outputFile).as(SUCCESS) } for { contents <- readSourceFile _ <- logAction(s"Compiling file: ${filePath.toAbsolutePath}") - outDir <- ensureOutputDir(outputDir.getOrElse(filePath.getParent)) - exitCode <- processProgram(contents, outDir) + exitCode <- processProgram(contents, outputDir.getOrElse(filePath.getParent)) } yield exitCode } -// TODO: Remove duplicate code between compileCommandSequential and compileCommandParallel -def compileCommandSequential( - files: NonEmptyList[Path], - log: Boolean, - outDir: Option[Path] -): IO[ExitCode] = - files - .traverse { file => - compile(file.toAbsolutePath, outDir, log).handleErrorWith { err => - // TODO: probably a more elegant way of doing this - // also, -1 arbitrary - // also - this outputs two messages for some reason - logger.error(err.getMessage) // *> IO.raiseError(err) - } - } - .map { exitCodes => - if (exitCodes.exists(_ != 0)) ExitCode.Error else ExitCode.Success - } - def compileCommandParallel( files: NonEmptyList[Path], log: Boolean, outDir: Option[Path] ): IO[ExitCode] = files - .parTraverse { file => - compile(file.toAbsolutePath, outDir, log).handleErrorWith { err => - // TODO: probably a more elegant way of doing this - // also, -1 arbitrary - // also - this outputs two messages for some reason - logger.error(err.getMessage) // *> IO.raiseError(err) - } - } + .parTraverse { file => compile(file.toAbsolutePath, outDir, log) } .map { exitCodes => - if (exitCodes.exists(_ != 0)) ExitCode.Error else ExitCode.Success + exitCodes.filter(_ != 0) match { + case Nil => ExitCode.Success + case errorCodes => ExitCode(errorCodes.min) + } } object Main @@ -183,9 +152,8 @@ object Main version = "1.0" ) { def main: Opts[IO[ExitCode]] = - (greedyOpt, filesOpt, logOpt, outputOpt).mapN { (greedy, files, log, outDir) => - if (greedy) compileCommandSequential(files, log, outDir) - else compileCommandParallel(files, log, outDir) + (filesOpt, logOpt, outputOpt).mapN { (files, log, outDir) => + compileCommandParallel(files, log, outDir) } } diff --git a/src/main/wacc/frontend/Error.scala b/src/main/wacc/frontend/Error.scala index b991421..e515494 100644 --- a/src/main/wacc/frontend/Error.scala +++ b/src/main/wacc/frontend/Error.scala @@ -3,6 +3,9 @@ package wacc import wacc.ast.Position import wacc.types._ +private val SYNTAX_ERROR = 100 +private val SEMANTIC_ERROR = 200 + /** Error types for semantic errors */ enum Error { @@ -14,6 +17,15 @@ enum Error { case SemanticError(pos: Position, msg: String) case TypeMismatch(pos: Position, expected: SemType, got: SemType, msg: String) case InternalError(pos: Position, msg: String) + + case SyntaxError(msg: String) +} + +extension (e: Error) { + def exitCode: Int = e match { + case Error.SyntaxError(_) => SYNTAX_ERROR + case _ => SEMANTIC_ERROR + } } /** Function to handle printing the details of a given semantic error @@ -25,7 +37,6 @@ enum Error { */ def formatError(error: Error)(using errorContent: String): String = { val sb = new StringBuilder() - sb.append("Semantic error:\n") /** Function to format the position of an error * @@ -55,6 +66,13 @@ def formatError(error: Error)(using errorContent: String): String = { ) } + error match { + case Error.SyntaxError(_) => + sb.append("Syntax error:\n") + case _ => + sb.append("Semantic error:\n") + } + error match { case Error.DuplicateDeclaration(ident) => formatPosition(ident.pos) @@ -87,6 +105,9 @@ def formatError(error: Error)(using errorContent: String): String = { formatPosition(pos) sb.append(s"Internal error: $msg") formatHighlight(pos, 1) + case Error.SyntaxError(msg) => + sb.append(msg) + sb.append("\n") } sb.toString() diff --git a/src/main/wacc/frontend/ast.scala b/src/main/wacc/frontend/ast.scala index ac0e585..9b14b13 100644 --- a/src/main/wacc/frontend/ast.scala +++ b/src/main/wacc/frontend/ast.scala @@ -4,7 +4,6 @@ import parsley.Parsley import parsley.generic.ErrorBridge import parsley.ap._ import parsley.position._ -import parsley.syntax.zipped._ import cats.data.NonEmptyList object ast {