From 665183a0c1ea6f43b91f02a938592d9a390cd7c5 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 3 Apr 2022 16:30:30 +0300 Subject: [PATCH 01/18] feat(analyzer.stateaccess): First working version of the analyzer --- .../odin/analysis/EOOdinAnalyzer.scala | 1 + .../analysis/stateaccess/DetectAccess.scala | 93 +++++++++++++++++++ .../odin/analysis/DetectAccessTests.scala | 71 ++++++++++++++ 3 files changed, 165 insertions(+) create mode 100644 analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala create mode 100644 analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala 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 f507daeb..edcbf9c4 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala @@ -9,6 +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.core.ast.EOProg import org.polystat.odin.core.ast.astparams.EOExprOnly import org.polystat.odin.parser.EoParser diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala new file mode 100644 index 00000000..7139b93a --- /dev/null +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectAccess.scala @@ -0,0 +1,93 @@ +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.parser.eo.Parser + +object DetectAccess { + + type ObjInfo = ObjectInfo[ParentInfo[MethodInfo, ObjectInfo], MethodInfo] + case class State(container: EONamedBnd, states: Vector[EONamedBnd]) + case class StateChange(method: EONamedBnd, state: EONamedBnd) + + def accumulateParentState(tree: Map[EONamedBnd, Inliner.CompleteObjectTree])( + currentParentLink: Option[ParentInfo[MethodInfo, ObjectInfo]], + existingState: Vector[EONamedBnd] = Vector() + ): List[State] = { + currentParentLink match { + case Some(parentLink) => + val parentObj = parentLink.linkToParent.getOption(tree).get + val currentStates = parentObj + .info + .bnds + .collect { + 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) + + case None => List() + } + } + + def getAlteredState(method: (EONamedBnd, MethodInfo)): List[StateChange] = { + val binds = method._2.body.bndAttrs + + Abstract.foldAst[List[StateChange]](binds) { + case EOCopy(EODot(EODot(EOSimpleAppWithLocator("self", x), state), "write"), _) if x == 0 => + List(StateChange(method._1, EOAnyNameBnd(LazyName(state)))) + } + } + + def analyze[F[_]]( + tree: Map[EONamedBnd, Inliner.CompleteObjectTree] + ): EitherNel[String, List[String]] = { + Right( + tree + // Has a parent + .filter(_._2.info.parentInfo.nonEmpty) + .flatMap(obj => { + val availableParentStates = + accumulateParentState(tree)(obj._2.info.parentInfo) + val alteredStates = obj._2.info.methods.flatMap(getAlteredState) + + for { + change <- alteredStates + State(container, states) <- availableParentStates + } yield + if (states.contains(change.state)) + List( + f"Method '${change.method.name.name}' of object '${obj._1.name.name}' directly accesses state '${change.state.name.name}' of base class '${container.name.name}'" + ) + else List() + }) + .flatten + .toList + ) + } + + def main(args: Array[String]): Unit = { + val 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 + + println( + Parser + .parse(code) + .flatMap(Inliner.createObjectTree) + .flatMap(Inliner.resolveParents) + .flatMap(analyze) + ) + } + +} diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala new file mode 100644 index 00000000..3729d53f --- /dev/null +++ b/analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala @@ -0,0 +1,71 @@ +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) + } + + } +} From e6f40b9c348d9eaedcef1dfcaef713f57d0dcd7e Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 3 Apr 2022 18:12:41 +0300 Subject: [PATCH 02/18] feat(analysis.stateaccess): integration with other analyzers --- .../odin/analysis/EOOdinAnalyzer.scala | 68 +++++++++++----- ...ctAccess.scala => DetectStateAccess.scala} | 36 +++++++-- .../odin/analysis/DetectAccessTests.scala | 71 ----------------- .../analysis/DetectStateAccessTests.scala | 78 +++++++++++++++++++ .../odin/interop/java/EOOdinAnalyzer.scala | 5 +- .../java/OdinAnalysisResultInterop.scala | 6 +- .../org/polystat/odin/sandbox/Sandbox.scala | 9 +++ 7 files changed, 173 insertions(+), 100 deletions(-) rename analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/{DetectAccess.scala => DetectStateAccess.scala} (78%) delete mode 100644 analysis/src/test/scala/org/polystat/odin/analysis/DetectAccessTests.scala create mode 100644 analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala 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 { From 9fe9bb18474620813388257d6e9aa80ffe1ec7f7 Mon Sep 17 00:00:00 2001 From: nikololiahim Date: Mon, 4 Apr 2022 15:18:54 +0300 Subject: [PATCH 03/18] minor fixes --- .../odin/analysis/EOOdinAnalyzer.scala | 12 +++--------- .../stateaccess/DetectStateAccess.scala | 19 ++----------------- .../analysis/DetectStateAccessTests.scala | 2 +- .../java/OdinAnalysisResultInterop.scala | 2 +- 4 files changed, 7 insertions(+), 28 deletions(-) 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 f89290ca..def09b3d 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/EOOdinAnalyzer.scala @@ -30,7 +30,7 @@ object EOOdinAnalyzer { final case class Ok(override val ruleId: String) extends OdinAnalysisResult - final case class DefectDetected( + final case class DefectsDetected( override val ruleId: String, messages: NonEmptyList[String], ) extends OdinAnalysisResult @@ -44,7 +44,7 @@ object EOOdinAnalyzer { analyzer: String )(errors: List[String]): OdinAnalysisResult = errors match { - case e :: es => DefectDetected(analyzer, NonEmptyList(e, es)) + case e :: es => DefectsDetected(analyzer, NonEmptyList(e, es)) case Nil => Ok(analyzer) } @@ -103,13 +103,7 @@ object EOOdinAnalyzer { stream .compile .toList - .map { - case Nil => OdinAnalysisResult.Ok(name) - case e :: es => OdinAnalysisResult.DefectDetected( - name, - NonEmptyList(e, es) - ) - } + .map(OdinAnalysisResult.fromErrors(name)) } } diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 6bbe5fdf..48d8b9fc 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -1,23 +1,8 @@ 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._ +import org.polystat.odin.core.ast._ import org.polystat.odin.parser.eo.Parser object DetectStateAccess { diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala index dff788d2..ef257af7 100644 --- a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala +++ b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala @@ -17,7 +17,7 @@ class DetectStateAccessTests extends AnyWordSpec { ) .flatMap { case Ok(_) => IO.pure(List.empty) - case DefectDetected(_, errors) => IO.pure(errors.toList) + case DefectsDetected(_, errors) => IO.pure(errors.toList) case AnalyzerFailure(_, e) => IO.raiseError(e) } 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 ba206115..fb044b1d 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 @@ -34,7 +34,7 @@ object OdinAnalysisResultInterop { java.util.Optional.empty, ) ) - case DefectDetected(rule, messages) => + case DefectsDetected(rule, messages) => List( new OdinAnalysisResultInterop( rule, From e0663ea52dd83a60096b8122a62a6e344243c39e Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 4 Apr 2022 15:27:21 +0300 Subject: [PATCH 04/18] docs(analysis.stateaccess) --- README.md | 2 + .../direct_access_to_base_class_state.md | 81 +++++++++++++++++++ docs/general_information.md | 4 + 3 files changed, 87 insertions(+) create mode 100644 docs/analysis/direct_access_to_base_class_state.md diff --git a/README.md b/README.md index cadb1413..5dd831bb 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,8 @@ Documentation for the mutual recursion analyzer is available [here](docs/analysi Documentation for the unjustified assumption analyzer is available [here](docs/analysis/unjustified_assumption_analyzer.md). +Documentation for the direct access to base class state analyzer is available [here](docs/analysis/direct_access_to_base_class_state.md). + # Running To run the project you will need [sbt](https://www.scala-sbt.org/1.x/docs/Setup.html) and JDK 8+. diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md new file mode 100644 index 00000000..c88a2485 --- /dev/null +++ b/docs/analysis/direct_access_to_base_class_state.md @@ -0,0 +1,81 @@ +# Direct Access to the Base Class State + +The fourth defect type suported by odin. + +## Original Problem Statement + +``` +An extension class should not access the state of its base +class directly, but only through calling base class methods. +``` + +Given the class `C` with methods `m` and `n`: +``` +C = class + x : int := 0; y : int := 0 + m => y := y + 1; x := y + n => y := y + 2; x := y +end +``` +And a potential modification `M` that affects method `n`: +``` +M = modifier + n => x:= x + 2 +end +``` +We will have the following: the modification `M`, if applied to class `C` might cause unnecessary confusion. The initial implemntation maintains an implicit invariant `x=y` by using `x := y` at the last action of every method. + +However, the redefinition of `n` in `M` causes the invariant to be broken in case of a calling the modified version of `n` after `m`. This will cause `y` to be equal to 1, and `x` to be equal to 3. + +Conversely, the sequence of calls `n;m` will cause a different kind of confusion. By looking at `C`, one could assume that the method calls will make `x` equal to 3, whereas, in fact, `x` will be assigned only 1. + +Thus, the best way to avoid such confusion is by only allowing changes to the variables defined in the base class to be made via the corresponding methods of the base class. + + +## EO Equivalnet of the Statement +In EO, base class state can be modelled with the use of the `memory` functionality. +So, having object `a` with state `state`: +``` +[] > a + memory > state + [self new_state] > update_state + seq > @ + state.write new_state + state +``` +The proper way to change the state in the subclass `b` would be: +``` +[] > b + a > @ + [self new_state] > change_state_plus_two + new_state.add 2 > tmp + seq > @ + self.update_state self tmp + state +``` +An **im**proper way to achieve the same functionality in subclass `bad`: +``` +[] > bad + a > @ + [self new_state] > change_state_plus_two + seq > @ + state.write (new_state.add 2) + state +``` + +## Brief description of the Devised algorithm +1. Build the `Tree` structure from the source code and resolve all parents. +2. Collect all existing subclasses. +3. Identify the state variables (ones that contain `memory`) accessible by each target subclass. +4. If any method uses the `write` on one of the parent's state variable - a message similar to the following is generated: + ` + Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a' + ` + +## Implementation Highlihts +1. ??? + + +## Current limitations +1. Only top-level objects are considered during analysis. +2. Some obscure ways to alter the state might not be accounted for??? \ No newline at end of file diff --git a/docs/general_information.md b/docs/general_information.md index 1d096909..b6ab80f0 100644 --- a/docs/general_information.md +++ b/docs/general_information.md @@ -12,6 +12,10 @@ Tool to inspect EO programs for potential defects. Additionally, it can be used > Assumptions made in subclasses regarding method dependencies in superclasses. (refer to [this document](analysis/unjustified_assumption_analyzer.md) for more information) +## 3. Direct access to base class state +> Altering the state stored in the base class in undesirable ways. + +(refer to [this document](analysis/direct_access_to_base_class_state.md) for more information) ## Assumptions and Limitations Some assumptions are made about EO programs and used EO constructs during analysis for all type of defects. Additionally, some constructs and syntax are not supported intentionally. From 67b5f6b50c188ae1d3e51a420217ddc97942abdd Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 10 Apr 2022 16:46:29 +0300 Subject: [PATCH 05/18] feat(analysis.stateAccess): support of `cage` as a possible state --- .../stateaccess/DetectStateAccess.scala | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 48d8b9fc..a8b3b775 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -1,10 +1,12 @@ package org.polystat.odin.analysis.stateaccess import cats.data.EitherNel +import cats.syntax.either._ import org.polystat.odin.analysis.inlining._ import org.polystat.odin.core.ast._ import org.polystat.odin.parser.eo.Parser + object DetectStateAccess { type ObjInfo = ObjectInfo[ParentInfo[MethodInfo, ObjectInfo], MethodInfo] @@ -23,7 +25,7 @@ object DetectStateAccess { .bnds .collect { case BndItself( - EOBndExpr(bndName, EOSimpleAppWithLocator("memory", _)) + EOBndExpr(bndName, EOSimpleAppWithLocator("memory" | "cage", _)) ) if !existingState.contains(bndName) => bndName } @@ -42,10 +44,7 @@ object DetectStateAccess { val binds = method._2.body.bndAttrs Abstract.foldAst[List[StateChange]](binds) { - case EOCopy( - EODot(EODot(EOSimpleAppWithLocator("self", x), state), "write"), - _ - ) if x == 0 => + case EODot(EOSimpleAppWithLocator("self", x), state) if x == 0 => List(StateChange(method._1, EOAnyNameBnd(LazyName(state)))) } } @@ -78,23 +77,37 @@ object DetectStateAccess { } def main(args: Array[String]): Unit = { - val code = """[] > a + val code = """ + |[] > super + | memory > explicit_state + | [] > super_state + | memory > hidden_state + | + |[] > a + | super > @ | memory > state | [self new_state] > update_state | self.state.write new_state > @ |[] > b | a > @ + | [self var] > alter_func + | var.write "bad" > @ + | [self] > bad_func + | self.alter_func self self.state > @ | [self new_state] > change_state_plus_two - | self.state.write (new_state.add 2) > @ + | self.bad_func self self.super_state.hidden_state > tmp + | self.explicit_state.write (new_state.add 2) > @ |""".stripMargin - println( + + Parser .parse(code) .flatMap(Inliner.createObjectTree) .flatMap(Inliner.resolveParents) .flatMap(analyze) - ) + .leftMap(println) + .foreach(_.foreach(println)) } } From 3df5cc54f695a23787bdd9e459f8b5aa4b5e4222 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 10 Apr 2022 16:48:28 +0300 Subject: [PATCH 06/18] fix(docs.stateAccess): Added `self` to examples --- docs/analysis/direct_access_to_base_class_state.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md index c88a2485..a7211b9f 100644 --- a/docs/analysis/direct_access_to_base_class_state.md +++ b/docs/analysis/direct_access_to_base_class_state.md @@ -40,8 +40,8 @@ So, having object `a` with state `state`: memory > state [self new_state] > update_state seq > @ - state.write new_state - state + self.state.write new_state + self.state ``` The proper way to change the state in the subclass `b` would be: ``` @@ -51,7 +51,7 @@ The proper way to change the state in the subclass `b` would be: new_state.add 2 > tmp seq > @ self.update_state self tmp - state + self.state ``` An **im**proper way to achieve the same functionality in subclass `bad`: ``` @@ -59,8 +59,8 @@ An **im**proper way to achieve the same functionality in subclass `bad`: a > @ [self new_state] > change_state_plus_two seq > @ - state.write (new_state.add 2) - state + self.state.write (new_state.add 2) + self.state ``` ## Brief description of the Devised algorithm From 69d67fb91f67addb3b2fa1624a2eef47ac56dca8 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 10 Apr 2022 17:10:42 +0300 Subject: [PATCH 07/18] docs(stateAccess): updated to match the latest specifications --- .../direct_access_to_base_class_state.md | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md index a7211b9f..3af53fe9 100644 --- a/docs/analysis/direct_access_to_base_class_state.md +++ b/docs/analysis/direct_access_to_base_class_state.md @@ -33,7 +33,9 @@ Thus, the best way to avoid such confusion is by only allowing changes to the va ## EO Equivalnet of the Statement -In EO, base class state can be modelled with the use of the `memory` functionality. +In EO, base class state can be modelled with the use of +`memory` functionality for variables and `cage` functionality for objects. + So, having object `a` with state `state`: ``` [] > a @@ -62,20 +64,52 @@ An **im**proper way to achieve the same functionality in subclass `bad`: self.state.write (new_state.add 2) self.state ``` +Access to state of any superclass is considered a bad practice. +Examples can be found in functions `read_func` and `bad_func`. + +> NOTE: access to local state, such as `local_state` is not considered harmful. +``` +[] > super_puper + memory > omega_state + +[] > super + super_puper > @ + memory > super_state + [self] > read_func + self.omega_state.add 2 > @ + +[] > parent + super > @ + memory > parent_state + +[] > child + parent > @ + memory > local_state + [self] > bad_func + seq > @ + self.omega_state.write 10 + self.super_state.write 10 + self.parent_state.write 10 + self.local_state.write 10 +``` ## Brief description of the Devised algorithm 1. Build the `Tree` structure from the source code and resolve all parents. 2. Collect all existing subclasses. -3. Identify the state variables (ones that contain `memory`) accessible by each target subclass. -4. If any method uses the `write` on one of the parent's state variable - a message similar to the following is generated: +3. Identify the state variables (ones that contain `memory` or `cage`) accessible by each target subclass. +4. If some method of the target subclass accesses a state variable present in its hierarchy - a message similar to the following is generated: ` Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a' ` ## Implementation Highlihts -1. ??? +1. Only variables that are accessed using the `self` object are considered. +So, `self.state.write 10` is considered, while `state.write 10` is not considered, +since it can potentially be a local variable. + ## Current limitations 1. Only top-level objects are considered during analysis. -2. Some obscure ways to alter the state might not be accounted for??? \ No newline at end of file +2. Some obscure ways to alter the state might not be accounted for ??? +3. Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file From f2d2e74d327a97c236c23ccc02a7242144724df9 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 10 Apr 2022 18:59:13 +0300 Subject: [PATCH 08/18] feat(analysis.stateAccess): support for defect detection in nested objects --- .../stateaccess/DetectStateAccess.scala | 84 ++++++++++++------- 1 file changed, 53 insertions(+), 31 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index a8b3b775..324ba82f 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -6,7 +6,6 @@ import org.polystat.odin.analysis.inlining._ import org.polystat.odin.core.ast._ import org.polystat.odin.parser.eo.Parser - object DetectStateAccess { type ObjInfo = ObjectInfo[ParentInfo[MethodInfo, ObjectInfo], MethodInfo] @@ -25,7 +24,10 @@ object DetectStateAccess { .bnds .collect { case BndItself( - EOBndExpr(bndName, EOSimpleAppWithLocator("memory" | "cage", _)) + EOBndExpr( + bndName, + EOSimpleAppWithLocator("memory" | "cage", _) + ) ) if !existingState.contains(bndName) => bndName } @@ -40,7 +42,7 @@ object DetectStateAccess { } } - def getAlteredState(method: (EONamedBnd, MethodInfo)): List[StateChange] = { + def getAccessedStates(method: (EONamedBnd, MethodInfo)): List[StateChange] = { val binds = method._2.body.bndAttrs Abstract.foldAst[List[StateChange]](binds) { @@ -49,31 +51,53 @@ object DetectStateAccess { } } - def analyze[F[_]]( + def detectStateAccesses( tree: Map[EONamedBnd, Inliner.CompleteObjectTree] + )(obj: (EONamedBnd, Inliner.CompleteObjectTree)): List[String] = { + val availableParentStates = + accumulateParentState(tree)(obj._2.info.parentInfo) + val accessedStates = obj._2.info.methods.flatMap(getAccessedStates) + val results = + for { + StateChange(changeSource, state) <- accessedStates + State(stateContainer, changedStates) <- availableParentStates + } yield + if (changedStates.contains(state)) { + val objName = obj._2.info.fqn.names.toList.mkString(".") + val stateName = state.name.name + val method = changeSource.name.name + val container = stateContainer.name.name + + List( + f"Method '${method}' of object '${objName}' directly accesses state '${stateName}' of base class '${container}'" + ) + } else List() + + results.toList.flatten + } + + def analyze[F[_]]( + originalTree: Map[EONamedBnd, Inliner.CompleteObjectTree] ): EitherNel[String, List[String]] = { - Right( + def helper( + tree: Map[EONamedBnd, Inliner.CompleteObjectTree] + ): List[String] = tree // Has a parent .filter(_._2.info.parentInfo.nonEmpty) - .flatMap(obj => { - val availableParentStates = - accumulateParentState(tree)(obj._2.info.parentInfo) - val alteredStates = obj._2.info.methods.flatMap(getAlteredState) - - for { - change <- alteredStates - State(container, states) <- availableParentStates - } yield - if (states.contains(change.state)) - List( - f"Method '${change.method.name.name}' of object '${obj._1.name.name}' directly accesses state '${change.state.name.name}' of base class '${container.name.name}'" - ) - else List() - }) - .flatten + .flatMap(detectStateAccesses(originalTree)) .toList - ) + + def recurse( + tree: Map[EONamedBnd, Inliner.CompleteObjectTree] + ): List[String] = { + val currentRes = helper(tree) + val children = tree.values.map(_.children) + + currentRes ++ children.flatMap(recurse) + } + + Right(recurse(originalTree)) } def main(args: Array[String]): Unit = { @@ -99,15 +123,13 @@ object DetectStateAccess { | self.explicit_state.write (new_state.add 2) > @ |""".stripMargin - - - Parser - .parse(code) - .flatMap(Inliner.createObjectTree) - .flatMap(Inliner.resolveParents) - .flatMap(analyze) - .leftMap(println) - .foreach(_.foreach(println)) + Parser + .parse(code) + .flatMap(Inliner.createObjectTree) + .flatMap(Inliner.resolveParents) + .flatMap(analyze) + .leftMap(println) + .foreach(_.foreach(println)) } } From ac59137242d6e3eb5d4ec677165ea25e22af72f1 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:02:06 +0300 Subject: [PATCH 09/18] feat(analysis.stateAccess): support for more obscure defect locations --- .../stateaccess/DetectStateAccess.scala | 112 +++++++++++++++--- 1 file changed, 94 insertions(+), 18 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 324ba82f..d9ecb7da 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -2,24 +2,62 @@ package org.polystat.odin.analysis.stateaccess import cats.data.EitherNel import cats.syntax.either._ +import higherkindness.droste.data.Fix import org.polystat.odin.analysis.inlining._ import org.polystat.odin.core.ast._ +import org.polystat.odin.core.ast.astparams.EOExprOnly import org.polystat.odin.parser.eo.Parser +import scala.annotation.tailrec + object DetectStateAccess { type ObjInfo = ObjectInfo[ParentInfo[MethodInfo, ObjectInfo], MethodInfo] - case class State(container: EONamedBnd, states: Vector[EONamedBnd]) - case class StateChange(method: EONamedBnd, state: EONamedBnd) + + case class State( + containerName: String, + statePath: List[String], + states: Vector[EONamedBnd] + ) + + case class StateChange( + method: EONamedBnd, + state: EONamedBnd, + statePath: List[String] + ) + + def collectNestedStates(mainParent: String)( + subTree: Inliner.CompleteObjectTree + ): Vector[State] = { + val currentLvlStateNames = subTree + .info + .bnds + .collect { + case BndItself( + EOBndExpr( + bndName, + EOSimpleAppWithLocator("memory" | "cage", _) + ) + ) => bndName + } + + Vector( + State(mainParent, subTree.info.fqn.names.tail, currentLvlStateNames) + ) ++ + subTree + .children + .flatMap(t => collectNestedStates(mainParent)(t._2)) + } def accumulateParentState(tree: Map[EONamedBnd, Inliner.CompleteObjectTree])( currentParentLink: Option[ParentInfo[MethodInfo, ObjectInfo]], - existingState: Vector[EONamedBnd] = Vector() - ): List[State] = { + existingStates: Vector[EONamedBnd] = Vector() + ): Vector[State] = { currentParentLink match { case Some(parentLink) => val parentObj = parentLink.linkToParent.getOption(tree).get - val currentStates = parentObj + val currentObjName = parentObj.info.name.name.name + val currentLvlStateNames = parentObj .info .bnds .collect { @@ -28,26 +66,65 @@ object DetectStateAccess { bndName, EOSimpleAppWithLocator("memory" | "cage", _) ) - ) if !existingState.contains(bndName) => + ) if !existingStates.contains(bndName) => bndName } + val currentLvlState = + State(currentObjName, List(), currentLvlStateNames) + val nestedStates = parentObj + .children + .flatMap(c => + collectNestedStates(parentObj.info.name.name.name)(c._2) + ) + .toVector - List(State(container = parentObj.info.name, states = currentStates)) ++ + Vector(currentLvlState) ++ nestedStates ++ accumulateParentState(tree)( parentObj.info.parentInfo, - existingState ++ currentStates + existingStates ++ currentLvlStateNames ) - case None => List() + case None => Vector() } } def getAccessedStates(method: (EONamedBnd, MethodInfo)): List[StateChange] = { + @tailrec + def hasSelfAsSource(dot: EODot[EOExprOnly]): Boolean = { + Fix.un(dot.src) match { + case EOSimpleAppWithLocator("self", x) if x == 0 => true + case innerDot @ EODot(_, _) => hasSelfAsSource(innerDot) + case _ => false + } + } + + def buildDotChain(dot: EODot[EOExprOnly]): List[String] = + Fix.un(dot.src) match { + case EOSimpleAppWithLocator("self", x) if x == 0 => List() + case innerDot @ EODot(_, _) => + buildDotChain(innerDot).appended(innerDot.name) + case _ => List() + } + val binds = method._2.body.bndAttrs + def processDot( + innerDot: EODot[Fix[EOExpr]], + state: String + ): List[StateChange] = { + val stateName = EOAnyNameBnd(LazyName(state)) + val containerChain = buildDotChain(innerDot) + + List(StateChange(method._1, stateName, containerChain)) + } + Abstract.foldAst[List[StateChange]](binds) { - case EODot(EOSimpleAppWithLocator("self", x), state) if x == 0 => - List(StateChange(method._1, EOAnyNameBnd(LazyName(state)))) + case EOCopy(Fix(dot @ EODot(Fix(innerDot @ EODot(_, state)), _)), _) + if hasSelfAsSource(dot) => + processDot(innerDot, state) + + case dot @ EODot(_, state) if hasSelfAsSource(dot) => + processDot(dot, state) } } @@ -59,17 +136,17 @@ object DetectStateAccess { val accessedStates = obj._2.info.methods.flatMap(getAccessedStates) val results = for { - StateChange(changeSource, state) <- accessedStates - State(stateContainer, changedStates) <- availableParentStates + StateChange(targetMethod, state, accessedStatePath) <- accessedStates + State(baseClass, statePath, changedStates) <- availableParentStates } yield - if (changedStates.contains(state)) { + if (changedStates.contains(state) && statePath == accessedStatePath) { val objName = obj._2.info.fqn.names.toList.mkString(".") val stateName = state.name.name - val method = changeSource.name.name - val container = stateContainer.name.name + val method = targetMethod.name.name + val container = statePath.prepended(baseClass).mkString(".") List( - f"Method '${method}' of object '${objName}' directly accesses state '${stateName}' of base class '${container}'" + f"Method '$method' of object '$objName' directly accesses state '$stateName' of base class '$container'" ) } else List() @@ -83,7 +160,6 @@ object DetectStateAccess { tree: Map[EONamedBnd, Inliner.CompleteObjectTree] ): List[String] = tree - // Has a parent .filter(_._2.info.parentInfo.nonEmpty) .flatMap(detectStateAccesses(originalTree)) .toList From 09d39c1bbc10e1b54b77090dd9493b092901a370 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:02:34 +0300 Subject: [PATCH 10/18] ref(analysis.stateAccess): remove unnecessary main method --- .../stateaccess/DetectStateAccess.scala | 33 ------------------- 1 file changed, 33 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index d9ecb7da..4e816cb9 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -175,37 +175,4 @@ object DetectStateAccess { Right(recurse(originalTree)) } - - def main(args: Array[String]): Unit = { - val code = """ - |[] > super - | memory > explicit_state - | [] > super_state - | memory > hidden_state - | - |[] > a - | super > @ - | memory > state - | [self new_state] > update_state - | self.state.write new_state > @ - |[] > b - | a > @ - | [self var] > alter_func - | var.write "bad" > @ - | [self] > bad_func - | self.alter_func self self.state > @ - | [self new_state] > change_state_plus_two - | self.bad_func self self.super_state.hidden_state > tmp - | self.explicit_state.write (new_state.add 2) > @ - |""".stripMargin - - Parser - .parse(code) - .flatMap(Inliner.createObjectTree) - .flatMap(Inliner.resolveParents) - .flatMap(analyze) - .leftMap(println) - .foreach(_.foreach(println)) - } - } From 16215650445970f31606dd46d0e01bc097c0cf6b Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:03:03 +0300 Subject: [PATCH 11/18] test(analysis.stateAccess): 12 test cases --- .../analysis/DetectStateAccessTests.scala | 174 +++++++++++++++++- 1 file changed, 173 insertions(+), 1 deletion(-) diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala index ef257af7..38c97a7a 100644 --- a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala +++ b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala @@ -23,7 +23,7 @@ class DetectStateAccessTests extends AnyWordSpec { val testsWithDefect: List[TestCase] = List( TestCase( - label = "Improper access to state", + label = "Write to state", code = """[] > a | memory > state | [self new_state] > update_state @@ -36,6 +36,155 @@ class DetectStateAccessTests extends AnyWordSpec { expected = List( "Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a'" ) + ), + TestCase( + label = "Access to state", + code = """[] > base + | memory > state + |[] > b + | base > @ + | [self var] > alter_var + | var.write 10 > @ + | [self] > change_state + | self.alter_var self self.state > @ + |""".stripMargin, + expected = List( + "Method 'change_state' of object 'b' directly accesses state 'state' of base class 'base'" + ) + ), + TestCase( + label = "Access to cage", + code = """[] > a + | cage > state + |[] > second_obj + | a > @ + | [self] > func + | self.state > @ + |""".stripMargin, + expected = List( + "Method 'func' of object 'second_obj' directly accesses state 'state' of base class 'a'" + ) + ), + TestCase( + label = "Access to cage AND memory", + code = """[] > a + | cage > state + | memory > mem + |[] > second_obj + | a > @ + | [self] > func + | self.state > tmp + | self.mem > @ + |""".stripMargin, + expected = List( + "Method 'func' of object 'second_obj' directly accesses state 'state' of base class 'a'", + "Method 'func' of object 'second_obj' directly accesses state 'mem' of base class 'a'" + ) + ), + TestCase( + label = "Access to inner state", + code = """[] > base + | memory > plain_state + | [] > inner_state + | [] > very_inner_state + | memory > hidden_state + | memory > inner_mem + | cage > inner_cage + |[] > b + | base > @ + | [self] > func + | self.inner_state.very_inner_state.hidden_state > super_tmp + | self.inner_state.inner_cage > tmp + | seq > @ + | self.plain_state.write 10 + | self.inner_state.inner_mem + |""".stripMargin, + expected = List( + "Method 'func' of object 'b' directly accesses state 'hidden_state' of base class 'base.inner_state.very_inner_state'", + "Method 'func' of object 'b' directly accesses state 'inner_cage' of base class 'base.inner_state'", + "Method 'func' of object 'b' directly accesses state 'plain_state' of base class 'base'", + "Method 'func' of object 'b' directly accesses state 'inner_mem' of base class 'base.inner_state'" + ) + ), + TestCase( + label = "Access to state in inner hierarchy", + code = """ + |[] > superroot + | [] > root + | [] > parent + | memory > state + | + | [] > child + | parent > @ + | [self] > method + | self.state.write 10 > @ + |""".stripMargin, + expected = List( + "Method 'method' of object 'superroot.root.child' directly accesses state 'state' of base class 'parent'", + ) + ), + TestCase( + label = "Access to state that is high in the hierarchy", + code = """ + |[] > super_puper + | memory > omega_state + | + |[] > super + | super_puper > @ + | memory > super_state + | [self] > super_bad_func + | seq > @ + | self.omega_state.write 10 + | self.super_state.write 30 + | + |[] > parent + | super > @ + | memory > parent_state + | + |[] > child + | parent > @ + | memory > local_state + | [self] > bad_func + | seq > @ + | self.omega_state.write 10 + | self.super_state.write 10 + | self.parent_state.write 10 + | self.local_state.write 10 + |""".stripMargin, + expected = List( + "Method 'super_bad_func' of object 'super' directly accesses state 'omega_state' of base class 'super_puper'", + "Method 'bad_func' of object 'child' directly accesses state 'omega_state' of base class 'super_puper'", + "Method 'bad_func' of object 'child' directly accesses state 'super_state' of base class 'super'", + "Method 'bad_func' of object 'child' directly accesses state 'parent_state' of base class 'parent'", + ) + ), + TestCase( + label = "Access to state with further method call", + code = """ + |[] > parent + | memory > state + |[] > child + | parent > @ + | [self] > method + | self.state.add 10 > @ + |""".stripMargin, + expected = List( + "Method 'method' of object 'child' directly accesses state 'state' of base class 'parent'", + ) + ), + TestCase( + label = "Access to state with a funky method call", + code = """ + |[] > parent + | memory > state + |[] > child + | parent > @ + | [self] > method + | 3.sub ((self.state.add 10).add 10) > @ + |""".stripMargin, + expected = List( + "Method 'method' of object 'child' directly accesses state 'state' of base class 'parent'", + ) ) ) @@ -53,6 +202,29 @@ class DetectStateAccessTests extends AnyWordSpec { | self.update_state self tmp > @ |""".stripMargin, expected = List() + ), + TestCase( + label = "State that is not accessed", + code = """[] > a + | memory > state + | memory > state_2 + | cage > obj_state + | [] > more_state + | memory > inner_state + |""".stripMargin, + expected = List() + ), + TestCase( + label = "Access to local state", + code = """[] > a + | memory > state + |[] > b + | a > @ + | memory > local_state + | [self] > func + | self.local_state.write 10 > @ + |""".stripMargin, + expected = List() ) ) From 22218d7d2c2961e32a070202716f8773b934c652 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:14:13 +0300 Subject: [PATCH 12/18] docs(analysis.stateAccess): updated to match latest functionality --- docs/analysis/direct_access_to_base_class_state.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md index 3af53fe9..f1621ab1 100644 --- a/docs/analysis/direct_access_to_base_class_state.md +++ b/docs/analysis/direct_access_to_base_class_state.md @@ -106,10 +106,9 @@ Examples can be found in functions `read_func` and `bad_func`. 1. Only variables that are accessed using the `self` object are considered. So, `self.state.write 10` is considered, while `state.write 10` is not considered, since it can potentially be a local variable. +2. The hierarchy where the defect takes place can be nested in objects. +3. - -## Current limitations -1. Only top-level objects are considered during analysis. -2. Some obscure ways to alter the state might not be accounted for ??? -3. Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file +## Possible limitations +1Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file From 41444b1cfd36265a236217de574c6c884e0753ac Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:14:38 +0300 Subject: [PATCH 13/18] ref(analysis.stateAccess): scalaFix --- .../odin/analysis/stateaccess/DetectStateAccess.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 4e816cb9..8f5281be 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -1,12 +1,11 @@ package org.polystat.odin.analysis.stateaccess import cats.data.EitherNel -import cats.syntax.either._ + import higherkindness.droste.data.Fix import org.polystat.odin.analysis.inlining._ import org.polystat.odin.core.ast._ import org.polystat.odin.core.ast.astparams.EOExprOnly -import org.polystat.odin.parser.eo.Parser import scala.annotation.tailrec @@ -175,4 +174,5 @@ object DetectStateAccess { Right(recurse(originalTree)) } + } From 5ea598e5866055b9541e89bdfb0a3b120b8cf058 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 11 Apr 2022 00:22:18 +0300 Subject: [PATCH 14/18] docs(analysis.stateAccess): minor fixes --- docs/analysis/direct_access_to_base_class_state.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md index f1621ab1..633cc333 100644 --- a/docs/analysis/direct_access_to_base_class_state.md +++ b/docs/analysis/direct_access_to_base_class_state.md @@ -107,8 +107,8 @@ Examples can be found in functions `read_func` and `bad_func`. So, `self.state.write 10` is considered, while `state.write 10` is not considered, since it can potentially be a local variable. 2. The hierarchy where the defect takes place can be nested in objects. -3. +3. Access to the state refers to actions that either read the state or call functions on it. ## Possible limitations -1Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file +1. Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file From 3722747bed417ab19dd1d2cd6dcccce23a12e839 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 8 May 2022 22:07:33 +0300 Subject: [PATCH 15/18] Revert "docs(analysis.stateAccess) --- README.md | 2 - .../direct_access_to_base_class_state.md | 114 ------------------ docs/general_information.md | 4 - 3 files changed, 120 deletions(-) delete mode 100644 docs/analysis/direct_access_to_base_class_state.md diff --git a/README.md b/README.md index 5dd831bb..cadb1413 100644 --- a/README.md +++ b/README.md @@ -13,8 +13,6 @@ Documentation for the mutual recursion analyzer is available [here](docs/analysi Documentation for the unjustified assumption analyzer is available [here](docs/analysis/unjustified_assumption_analyzer.md). -Documentation for the direct access to base class state analyzer is available [here](docs/analysis/direct_access_to_base_class_state.md). - # Running To run the project you will need [sbt](https://www.scala-sbt.org/1.x/docs/Setup.html) and JDK 8+. diff --git a/docs/analysis/direct_access_to_base_class_state.md b/docs/analysis/direct_access_to_base_class_state.md deleted file mode 100644 index 633cc333..00000000 --- a/docs/analysis/direct_access_to_base_class_state.md +++ /dev/null @@ -1,114 +0,0 @@ -# Direct Access to the Base Class State - -The fourth defect type suported by odin. - -## Original Problem Statement - -``` -An extension class should not access the state of its base -class directly, but only through calling base class methods. -``` - -Given the class `C` with methods `m` and `n`: -``` -C = class - x : int := 0; y : int := 0 - m => y := y + 1; x := y - n => y := y + 2; x := y -end -``` -And a potential modification `M` that affects method `n`: -``` -M = modifier - n => x:= x + 2 -end -``` -We will have the following: the modification `M`, if applied to class `C` might cause unnecessary confusion. The initial implemntation maintains an implicit invariant `x=y` by using `x := y` at the last action of every method. - -However, the redefinition of `n` in `M` causes the invariant to be broken in case of a calling the modified version of `n` after `m`. This will cause `y` to be equal to 1, and `x` to be equal to 3. - -Conversely, the sequence of calls `n;m` will cause a different kind of confusion. By looking at `C`, one could assume that the method calls will make `x` equal to 3, whereas, in fact, `x` will be assigned only 1. - -Thus, the best way to avoid such confusion is by only allowing changes to the variables defined in the base class to be made via the corresponding methods of the base class. - - -## EO Equivalnet of the Statement -In EO, base class state can be modelled with the use of -`memory` functionality for variables and `cage` functionality for objects. - -So, having object `a` with state `state`: -``` -[] > a - memory > state - [self new_state] > update_state - seq > @ - self.state.write new_state - self.state -``` -The proper way to change the state in the subclass `b` would be: -``` -[] > b - a > @ - [self new_state] > change_state_plus_two - new_state.add 2 > tmp - seq > @ - self.update_state self tmp - self.state -``` -An **im**proper way to achieve the same functionality in subclass `bad`: -``` -[] > bad - a > @ - [self new_state] > change_state_plus_two - seq > @ - self.state.write (new_state.add 2) - self.state -``` -Access to state of any superclass is considered a bad practice. -Examples can be found in functions `read_func` and `bad_func`. - -> NOTE: access to local state, such as `local_state` is not considered harmful. -``` -[] > super_puper - memory > omega_state - -[] > super - super_puper > @ - memory > super_state - [self] > read_func - self.omega_state.add 2 > @ - -[] > parent - super > @ - memory > parent_state - -[] > child - parent > @ - memory > local_state - [self] > bad_func - seq > @ - self.omega_state.write 10 - self.super_state.write 10 - self.parent_state.write 10 - self.local_state.write 10 -``` - -## Brief description of the Devised algorithm -1. Build the `Tree` structure from the source code and resolve all parents. -2. Collect all existing subclasses. -3. Identify the state variables (ones that contain `memory` or `cage`) accessible by each target subclass. -4. If some method of the target subclass accesses a state variable present in its hierarchy - a message similar to the following is generated: - ` - Method 'change_state_plus_two' of object 'b' directly accesses state 'state' of base class 'a' - ` - -## Implementation Highlihts -1. Only variables that are accessed using the `self` object are considered. -So, `self.state.write 10` is considered, while `state.write 10` is not considered, -since it can potentially be a local variable. -2. The hierarchy where the defect takes place can be nested in objects. -3. Access to the state refers to actions that either read the state or call functions on it. - - -## Possible limitations -1. Variables can be accessed without using `self`. The current implementation does not consider such cases. \ No newline at end of file diff --git a/docs/general_information.md b/docs/general_information.md index b6ab80f0..1d096909 100644 --- a/docs/general_information.md +++ b/docs/general_information.md @@ -12,10 +12,6 @@ Tool to inspect EO programs for potential defects. Additionally, it can be used > Assumptions made in subclasses regarding method dependencies in superclasses. (refer to [this document](analysis/unjustified_assumption_analyzer.md) for more information) -## 3. Direct access to base class state -> Altering the state stored in the base class in undesirable ways. - -(refer to [this document](analysis/direct_access_to_base_class_state.md) for more information) ## Assumptions and Limitations Some assumptions are made about EO programs and used EO constructs during analysis for all type of defects. Additionally, some constructs and syntax are not supported intentionally. From 03deacfa62e2b3793bd44c8041d0b2ce783c5897 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Mon, 9 May 2022 14:49:51 +0300 Subject: [PATCH 16/18] test(analysis.directAccess): even more tests --- .../analysis/DetectStateAccessTests.scala | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala index 38c97a7a..4870c4d5 100644 --- a/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala +++ b/analysis/src/test/scala/org/polystat/odin/analysis/DetectStateAccessTests.scala @@ -52,6 +52,98 @@ class DetectStateAccessTests extends AnyWordSpec { "Method 'change_state' of object 'b' directly accesses state 'state' of base class 'base'" ) ), + TestCase( + label = "calculation chain", + code = """[] > test + | [] > a + | memory > state + | [] > b + | a > @ + | [self x] > n + | add. > @ + | x + | mul. + | 100 + | add. + | 100 + | sub. + | 100 + | self.state + |""".stripMargin, + expected = List( + "Method 'n' of object 'test.b' directly accesses state 'state' of base class 'a'" + ) + ), + TestCase( + label = "read-in-calculation-chain", + code = """[] > test + | [] > a + | memory > state + | [] > b + | a > @ + | [self x] > n + | add. > @ + | self.state + | mul. + | 100 + | add. + | 100 + | sub. + | 100 + | x + |""".stripMargin, + expected = List( + "Method 'n' of object 'test.b' directly accesses state 'state' of base class 'a'" + ) + ), + TestCase( + label = "read-in-inheritance-chain", + code = """[] > test + | [] > a + | memory > state + | [] > b + | a > @ + | [] > c + | b > @ + | [self x] > n + | self.state.add x > @ + |""".stripMargin, + expected = List( + "Method 'n' of object 'test.c' directly accesses state 'state' of base class 'a'" + ) + ), + TestCase( + label = "access-read-nested-class-2", + code = """[] > test + | [] > very_outer + | [] > outer + | [] > a + | memory > state + | [] > b + | a > @ + | [self x] > n + | self.state.add x > @ + |""".stripMargin, + expected = List( + "Method 'n' of object 'test.very_outer.outer.b' directly accesses state 'state' of base class 'a'" + ) + ), + TestCase( + label = "write-through-another-method", + code = """[] > test + | [] > a + | memory > state + | [] > b + | a > @ + | [self x y] > m + | x.write y > @ + | [self y] > n + | self.m self (self.state) y > @ + |""".stripMargin, + expected = List( + "Method 'n' of object 'test.b' directly accesses state 'state' of base class 'a'" + ) + ), TestCase( label = "Access to cage", code = """[] > a @@ -158,6 +250,40 @@ class DetectStateAccessTests extends AnyWordSpec { "Method 'bad_func' of object 'child' directly accesses state 'parent_state' of base class 'parent'", ) ), + TestCase( + label = + "Access to state that is high in the hierarchy | shadowing, attaching attributes during application", + code = """ + |[] > super_puper + | memory > omega_state + | memory > additional_state + | + |[] > super + | super_puper > @ + | super_puper.additional_state > omega_state + | [self] > super_bad_func + | seq > @ + | self.omega_state.write 10 + | + |[] > parent + | ((super)) > @ + | memory > parent_state + | + |[] > child + | parent > @ + | memory > local_state + | [self] > bad_func + | seq > @ + | self.omega_state.write 10 + | self.parent_state.write 10 + | self.local_state.write 10 + |""".stripMargin, + expected = List( + "Method 'super_bad_func' of object 'super' directly accesses state 'omega_state' of base class 'super_puper'", + "Method 'bad_func' of object 'child' directly accesses state 'omega_state' of base class 'super_puper'", + "Method 'bad_func' of object 'child' directly accesses state 'parent_state' of base class 'parent'" + ) + ), TestCase( label = "Access to state with further method call", code = """ @@ -172,6 +298,20 @@ class DetectStateAccessTests extends AnyWordSpec { "Method 'method' of object 'child' directly accesses state 'state' of base class 'parent'", ) ), + TestCase( + label = "Access to state with further method call | indirect", + code = """ + |[] > parent + | memory > state + |[] > child + | parent > @ + | [self] > method + | (self.state.add 10).write 4 > @ + |""".stripMargin, + expected = List( + "Method 'method' of object 'child' directly accesses state 'state' of base class 'parent'", + ) + ), TestCase( label = "Access to state with a funky method call", code = """ @@ -203,6 +343,19 @@ class DetectStateAccessTests extends AnyWordSpec { |""".stripMargin, expected = List() ), + TestCase( + label = "Read on not inherited object", + code = """[] > test + | [] > a_factory + | [] > get_a + | memory > state + | [] > b + | a_factory.get_a > @ + | [self x] > n + | a_factory.get_a.state.add x > @ + |""".stripMargin, + expected = List() + ), TestCase( label = "State that is not accessed", code = """[] > a From bb842559dc82e365bb756fb575a0125a8e72fb74 Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 15 May 2022 21:59:02 +0300 Subject: [PATCH 17/18] Fix(analysis.Results): merge conflicts --- .../odin/analysis/stateaccess/DetectStateAccess.scala | 4 ++-- .../polystat/odin/analysis/LiskovPrincipleTests.scala | 2 +- .../odin/analysis/UnjustifiedAssumptionTests.scala | 2 +- .../org/polystat/odin/interop/java/EOOdinAnalyzer.scala | 9 +-------- 4 files changed, 5 insertions(+), 12 deletions(-) diff --git a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala index 8f5281be..2e4a7f2b 100644 --- a/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala +++ b/analysis/src/main/scala/org/polystat/odin/analysis/stateaccess/DetectStateAccess.scala @@ -1,9 +1,9 @@ package org.polystat.odin.analysis.stateaccess import cats.data.EitherNel - import higherkindness.droste.data.Fix -import org.polystat.odin.analysis.inlining._ +import org.polystat.odin.analysis.utils.Abstract +import org.polystat.odin.analysis.utils.inlining._ import org.polystat.odin.core.ast._ import org.polystat.odin.core.ast.astparams.EOExprOnly diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/LiskovPrincipleTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/LiskovPrincipleTests.scala index 3e6790ed..810f5026 100644 --- a/analysis/src/test/scala/org/polystat/odin/analysis/LiskovPrincipleTests.scala +++ b/analysis/src/test/scala/org/polystat/odin/analysis/LiskovPrincipleTests.scala @@ -18,7 +18,7 @@ class LiskovPrincipleTests extends AnyWordSpec { ) .flatMap { case Ok(_) => IO.pure(List.empty) - case DefectDetected(_, message) => IO.pure(message.split("\n").toList) + case DefectsDetected(_, message) => IO.pure(message.toList) case AnalyzerFailure(_, e) => IO.raiseError(e) } diff --git a/analysis/src/test/scala/org/polystat/odin/analysis/UnjustifiedAssumptionTests.scala b/analysis/src/test/scala/org/polystat/odin/analysis/UnjustifiedAssumptionTests.scala index 28883200..5555ba8e 100644 --- a/analysis/src/test/scala/org/polystat/odin/analysis/UnjustifiedAssumptionTests.scala +++ b/analysis/src/test/scala/org/polystat/odin/analysis/UnjustifiedAssumptionTests.scala @@ -18,7 +18,7 @@ class UnjustifiedAssumptionTests extends AnyWordSpec { ) .flatMap { case Ok(_) => IO.pure(List.empty) - case DefectDetected(_, message) => IO.pure(message.split("\n").toList) + case DefectsDetected(_, message) => IO.pure(message.toList) case AnalyzerFailure(_, e) => IO.raiseError(e) } 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 7bc452e8..99e70831 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 @@ -5,19 +5,12 @@ import cats.effect.unsafe.IORuntime import cats.syntax.all._ import org.polystat.odin.analysis import org.polystat.odin.analysis.ASTAnalyzer -import org.polystat.odin.analysis.EOOdinAnalyzer.{ - advancedMutualRecursionAnalyzer, - directStateAccessAnalyzer, - liskovPrincipleViolationAnalyzer, - unjustifiedAssumptionAnalyzer -} +import org.polystat.odin.analysis.EOOdinAnalyzer.{advancedMutualRecursionAnalyzer, directStateAccessAnalyzer, liskovPrincipleViolationAnalyzer, 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._ From 8f36dc41d6e966f5e6360b9cdc43bccdb38110ee Mon Sep 17 00:00:00 2001 From: Vitaliy <42554566+Leosimetti@users.noreply.github.com> Date: Sun, 15 May 2022 22:02:20 +0300 Subject: [PATCH 18/18] Ref(all): scalafmt --- .../org/polystat/odin/interop/java/EOOdinAnalyzer.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 99e70831..c5f62195 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 @@ -5,7 +5,12 @@ import cats.effect.unsafe.IORuntime import cats.syntax.all._ import org.polystat.odin.analysis import org.polystat.odin.analysis.ASTAnalyzer -import org.polystat.odin.analysis.EOOdinAnalyzer.{advancedMutualRecursionAnalyzer, directStateAccessAnalyzer, liskovPrincipleViolationAnalyzer, unjustifiedAssumptionAnalyzer} +import org.polystat.odin.analysis.EOOdinAnalyzer.{ + advancedMutualRecursionAnalyzer, + directStateAccessAnalyzer, + liskovPrincipleViolationAnalyzer, + unjustifiedAssumptionAnalyzer +} import org.polystat.odin.core.ast.EOProg import org.polystat.odin.core.ast.astparams.EOExprOnly import org.polystat.odin.parser.EoParser