diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala b/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala index edcbf9c4..f89290ca 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala @@ -1,7 +1,7 @@ package org.polystat.odin.analysis import cats._ -import cats.data.EitherNel +import cats.data.{EitherNel, NonEmptyList} import cats.effect.Sync import cats.syntax.all._ import fs2.Stream @@ -9,7 +9,7 @@ import org.polystat.odin.analysis.inlining.Inliner import org.polystat.odin.analysis.logicalexprs.ExtractLogic import org.polystat.odin.analysis.mutualrec.advanced.Analyzer.analyzeAst import org.polystat.odin.analysis.mutualrec.naive.findMutualRecursionFromAst -import org.polystat.odin.analysis.stateaccess.DetectAccess +import org.polystat.odin.analysis.stateaccess.DetectStateAccess import org.polystat.odin.core.ast.EOProg import org.polystat.odin.core.ast.astparams.EOExprOnly import org.polystat.odin.parser.EoParser @@ -32,7 +32,7 @@ object EOOdinAnalyzer { final case class DefectDetected( override val ruleId: String, - message: String + messages: NonEmptyList[String], ) extends OdinAnalysisResult final case class AnalyzerFailure( @@ -43,10 +43,10 @@ object EOOdinAnalyzer { def fromErrors( analyzer: String )(errors: List[String]): OdinAnalysisResult = - if (errors.isEmpty) - Ok(analyzer) - else - DefectDetected(analyzer, errors.mkString("\n")) + errors match { + case e :: es => DefectDetected(analyzer, NonEmptyList(e, es)) + case Nil => Ok(analyzer) + } def fromThrow[F[_]: ApplicativeThrow]( analyzer: String @@ -58,6 +58,16 @@ object EOOdinAnalyzer { } + private def toThrow[F[_], A]( + eitherNel: EitherNel[String, A] + )(implicit mt: MonadThrow[F]): F[A] = { + MonadThrow[F].fromEither( + eitherNel + .leftMap(_.mkString_(util.Properties.lineSeparator)) + .leftMap(new Exception(_)) + ) + } + def naiveMutualRecursionAnalyzer[F[_]: Sync]: ASTAnalyzer[F] = new ASTAnalyzer[F] { @@ -90,7 +100,16 @@ object EOOdinAnalyzer { }) } yield odinError - stream.compile.toList.map(OdinAnalysisResult.fromErrors(name)) + stream + .compile + .toList + .map { + case Nil => OdinAnalysisResult.Ok(name) + case e :: es => OdinAnalysisResult.DefectDetected( + name, + NonEmptyList(e, es) + ) + } } } @@ -118,14 +137,6 @@ object EOOdinAnalyzer { override val name: String = "Unjustified Assumption" - private def toThrow[A](eitherNel: EitherNel[String, A]): F[A] = { - MonadThrow[F].fromEither( - eitherNel - .leftMap(_.mkString_(util.Properties.lineSeparator)) - .leftMap(new Exception(_)) - ) - } - override def analyze( ast: EOProg[EOExprOnly] ): F[OdinAnalysisResult] = @@ -140,12 +151,33 @@ object EOOdinAnalyzer { } - def analyzeSourceCode[EORepr, F[_]: Monad]( + def directStateAccessAnalyzer[F[_]: MonadThrow]: ASTAnalyzer[F] = + new ASTAnalyzer[F] { + + override val name: String = "Direct Access to Superclass State" + + override def analyze( + ast: EOProg[EOExprOnly] + ): F[OdinAnalysisResult] = + OdinAnalysisResult.fromThrow[F](name) { + for { + tmpTree <- + toThrow(Inliner.createObjectTree(ast)) + tree <- toThrow(Inliner.resolveParents(tmpTree)) + errors <- + toThrow(DetectStateAccess.analyze(tree)) + } yield errors + } + + } + + def analyzeSourceCode[EORepr, F[_]]( analyzer: ASTAnalyzer[F] )( eoRepr: EORepr )(implicit - parser: EoParser[EORepr, F, EOProg[EOExprOnly]] + m: Monad[F], + parser: EoParser[EORepr, F, EOProg[EOExprOnly]], ): F[OdinAnalysisResult] = for { programAst <- parser.parse(eoRepr) mutualRecursionErrors <- diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala similarity index 78% rename from analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala rename to analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 7139b93a..6bbe5fdf 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -1,11 +1,26 @@ package org.polystat.odin.analysis.stateaccess import cats.data.EitherNel -import org.polystat.odin.analysis.inlining.{Abstract, BndItself, Inliner, MethodInfo, ObjectInfo, ParentInfo} -import org.polystat.odin.core.ast.{EOAnyNameBnd, EOBndExpr, EOCopy, EODot, EONamedBnd, EOSimpleAppWithLocator, LazyName} +import org.polystat.odin.analysis.inlining.{ + Abstract, + BndItself, + Inliner, + MethodInfo, + ObjectInfo, + ParentInfo +} +import org.polystat.odin.core.ast.{ + EOAnyNameBnd, + EOBndExpr, + EOCopy, + EODot, + EONamedBnd, + EOSimpleAppWithLocator, + LazyName +} import org.polystat.odin.parser.eo.Parser -object DetectAccess { +object DetectStateAccess { type ObjInfo = ObjectInfo[ParentInfo[MethodInfo, ObjectInfo], MethodInfo] case class State(container: EONamedBnd, states: Vector[EONamedBnd]) @@ -22,13 +37,17 @@ object DetectAccess { .info .bnds .collect { - case BndItself(EOBndExpr(bndName, EOSimpleAppWithLocator("memory", _))) - if !existingState.contains(bndName) => + case BndItself( + EOBndExpr(bndName, EOSimpleAppWithLocator("memory", _)) + ) if !existingState.contains(bndName) => bndName } List(State(container = parentObj.info.name, states = currentStates)) ++ - accumulateParentState(tree)(parentObj.info.parentInfo, existingState ++ currentStates) + accumulateParentState(tree)( + parentObj.info.parentInfo, + existingState ++ currentStates + ) case None => List() } @@ -38,7 +57,10 @@ object DetectAccess { val binds = method._2.body.bndAttrs Abstract.foldAst[List[StateChange]](binds) { - case EOCopy(EODot(EODot(EOSimpleAppWithLocator("self", x), state), "write"), _) if x == 0 => + case EOCopy( + EODot(EODot(EOSimpleAppWithLocator("self", x), state), "write"), + _ + ) if x == 0 => List(StateChange(method._1, EOAnyNameBnd(LazyName(state)))) } } diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala deleted file mode 100644 index 3729d53f..00000000 --- a/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala +++ /dev/null @@ -1,71 +0,0 @@ -package org.polystat.odin.analysis - -import cats.effect._ -import org.scalatest.wordspec.AnyWordSpec -import org.polystat.odin.parser.EoParser.sourceCodeEoParser -import cats.effect.unsafe.implicits.global -import org.polystat.odin.analysis.EOOdinAnalyzer.accessToBaseClassAnalyzer - -class DetectAccessTests extends AnyWordSpec { - case class TestCase(label: String, code: String, expected: List[String]) - - def analyze(code: String): IO[List[String]] = - EOOdinAnalyzer - .analyzeSourceCode[String, IO](accessToBaseClassAnalyzer[IO])(code)(sourceCodeEoParser()) - .compile - .toList - .map(_.map(_.value)) - - val testsWithDefect = List( - TestCase( - label = "Improper access to state", - code = """[] > a - | memory > state - | [self new_state] > update_state - | self.state.write new_state > @ - |[] > b - | a > @ - | [self new_state] > change_state_plus_two - | self.state.write (new_state.add 2) > @ - |""".stripMargin, - expected = List("Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a'") - ) - ) - - val testsWithoutDefect = List( - TestCase( - label = "Proper access to state", - code = """[] > a - | memory > state - | [self new_state] > update_state - | self.state.write new_state > @ - |[] > b - | a > @ - | [self new_state] > change_state_plus_two - | new_state.add 2 > tmp - | self.update_state self tmp > @ - |""".stripMargin, - expected = List() - ) - ) - - def runTests(tests: List[TestCase]) : Unit = - tests.foreach { - case TestCase(label, code, expected) => - registerTest(label) { - val obtained = analyze(code).unsafeRunSync() - assert(obtained == expected) - } - } - - "analyzer" should { - "find errors" should { - runTests(testsWithDefect) - } - - "not find errors" should { - runTests(testsWithoutDefect) - } - - } -} diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala new file mode 100644 index 00000000..dff788d2 --- /dev/null +++ b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala @@ -0,0 +1,78 @@ +package org.polystat.odin.analysis + +import cats.effect._ +import org.scalatest.wordspec.AnyWordSpec +import org.polystat.odin.parser.EoParser.sourceCodeEoParser +import cats.effect.unsafe.implicits.global +import org.polystat.odin.analysis.EOOdinAnalyzer.directStateAccessAnalyzer +import EOOdinAnalyzer.OdinAnalysisResult._ + +class DetectStateAccessTests extends AnyWordSpec { + case class TestCase(label: String, code: String, expected: List[String]) + + def analyze(code: String): IO[List[String]] = EOOdinAnalyzer + .analyzeSourceCode[String, IO](directStateAccessAnalyzer)(code)( + cats.Monad[IO], + sourceCodeEoParser() + ) + .flatMap { + case Ok(_) => IO.pure(List.empty) + case DefectDetected(_, errors) => IO.pure(errors.toList) + case AnalyzerFailure(_, e) => IO.raiseError(e) + } + + val testsWithDefect: List[TestCase] = List( + TestCase( + label = "Improper access to state", + code = """[] > a + | memory > state + | [self new_state] > update_state + | self.state.write new_state > @ + |[] > b + | a > @ + | [self new_state] > change_state_plus_two + | self.state.write (new_state.add 2) > @ + |""".stripMargin, + expected = List( + "Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a'" + ) + ) + ) + + val testsWithoutDefect: List[TestCase] = List( + TestCase( + label = "Proper access to state", + code = """[] > a + | memory > state + | [self new_state] > update_state + | self.state.write new_state > @ + |[] > b + | a > @ + | [self new_state] > change_state_plus_two + | new_state.add 2 > tmp + | self.update_state self tmp > @ + |""".stripMargin, + expected = List() + ) + ) + + def runTests(tests: List[TestCase]): Unit = + tests.foreach { case TestCase(label, code, expected) => + registerTest(label) { + val obtained = analyze(code).unsafeRunSync() + assert(obtained == expected) + } + } + + "analyzer" should { + "find errors" should { + runTests(testsWithDefect) + } + + "not find errors" should { + runTests(testsWithoutDefect) + } + + } + +} diff --git a/interop/src/main/scala/org/polystat/odin/interop/java/EOOdinAnalyzer.scala b/interop/src/main/scala/org/polystat/odin/interop/java/EOOdinAnalyzer.scala index dae5a15a..71973823 100644 --- a/interop/src/main/scala/org/polystat/odin/interop/java/EOOdinAnalyzer.scala +++ b/interop/src/main/scala/org/polystat/odin/interop/java/EOOdinAnalyzer.scala @@ -7,16 +7,16 @@ import org.polystat.odin.analysis import org.polystat.odin.analysis.ASTAnalyzer import org.polystat.odin.analysis.EOOdinAnalyzer.{ advancedMutualRecursionAnalyzer, + directStateAccessAnalyzer, unjustifiedAssumptionAnalyzer } import org.polystat.odin.core.ast.EOProg import org.polystat.odin.core.ast.astparams.EOExprOnly import org.polystat.odin.parser.EoParser import org.polystat.odin.parser.EoParser.sourceCodeEoParser -import org.polystat.odin.interop.java.OdinAnalysisResultInterop -import scala.jdk.CollectionConverters._ import java.util +import scala.jdk.CollectionConverters._ trait EOOdinAnalyzer[R] { @@ -33,6 +33,7 @@ object EOOdinAnalyzer { List( advancedMutualRecursionAnalyzer[IO], unjustifiedAssumptionAnalyzer[IO], + directStateAccessAnalyzer[IO], ) private def runAnalyzers( diff --git a/interop/src/main/scala/org/polystat/odin/interop/java/OdinAnalysisResultInterop.scala b/interop/src/main/scala/org/polystat/odin/interop/java/OdinAnalysisResultInterop.scala index f10ac433..ba206115 100644 --- a/interop/src/main/scala/org/polystat/odin/interop/java/OdinAnalysisResultInterop.scala +++ b/interop/src/main/scala/org/polystat/odin/interop/java/OdinAnalysisResultInterop.scala @@ -2,6 +2,8 @@ package org.polystat.odin.interop.java import org.polystat.odin.analysis.EOOdinAnalyzer.OdinAnalysisResult import org.polystat.odin.analysis.EOOdinAnalyzer.OdinAnalysisResult._ +import cats.syntax.foldable._ +import scala.util.Properties class OdinAnalysisResultInterop( val ruleId: java.lang.String, @@ -32,11 +34,11 @@ object OdinAnalysisResultInterop { java.util.Optional.empty, ) ) - case DefectDetected(rule, message) => + case DefectDetected(rule, messages) => List( new OdinAnalysisResultInterop( rule, - java.util.Optional.of(message), + java.util.Optional.of(messages.mkString_(Properties.lineSeparator)), java.util.Optional.empty, ) ) diff --git a/sandbox/src/main/scala/org/polystat/odin/sandbox/Sandbox.scala b/sandbox/src/main/scala/org/polystat/odin/sandbox/Sandbox.scala index 38314c7a..f8b69855 100644 --- a/sandbox/src/main/scala/org/polystat/odin/sandbox/Sandbox.scala +++ b/sandbox/src/main/scala/org/polystat/odin/sandbox/Sandbox.scala @@ -64,6 +64,15 @@ object Sandbox extends IOApp { | [self] > h | self.f self 1 2 3 > @ |""".stripMargin, + "eight" -> """[] > a + | memory > state + | [self new_state] > update_state + | self.state.write new_state > @ + |[] > b + | a > @ + | [self new_state] > change_state_plus_two + | self.state.write (new_state.add 2) > @ + |""".stripMargin, ) override def run(args: List[String]): IO[ExitCode] = for {