feat(analysis.stateaccess): integration with other analyzers

analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala

@@ -1,15 +1,15 @@
 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
 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 <-

analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala → 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))))
     }
   }

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)
+    }
+
+  }
+
+}

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(

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,
           )
         )