Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check cycles on package level #141

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

debs-sifive
Copy link

Include test which proves that force flag does not enforce package level checks

Rebase of #119 by jackkoenig which is a rebase of #30 originally by ghostbuster91

@debs-sifive debs-sifive marked this pull request as ready for review October 29, 2024 17:31
Comment on lines +58 to +65
private val pkgAccumulator = new tpd.TreeAccumulator[List[tpd.Tree]] {
def apply(acc: List[tpd.Tree], tree: tpd.Tree)(using Context): List[tpd.Tree] =
foldOver(
if (tree.symbol.is(Package) && tree.symbol.fullName.toString != "<empty>") tree :: acc else acc,
tree
)
}

Copy link
Author

@debs-sifive debs-sifive Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied the pattern in pkgObjectAccumulator for this, but I'm not sure that the behavior is correct or efficient. The packages appear to be repeated many more times with the foldOver compared to collect. This is how the packages compare between this Scala 3 plugin and the Scala 2 plugin:

Scala 3

stdPackages
        Pkg(List(forcepkg, cyclicpackage, a))
        Pkg(List(forcepkg, cyclicpackage, a))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg))
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg))
        Pkg(List(scala, annotation)) <-- idk where these are coming from
        Pkg(List(scala))             <-- or if they should be here at all
        Pkg(List(acyclic))
        Pkg(List(acyclic))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg))
        Pkg(List(forcepkg, cyclicpackage, a))
        Pkg(List(forcepkg, cyclicpackage, a))

Scala 2

stdPackages
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage, a))
        Pkg(List(acyclic))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg, cyclicpackage, b))
        Pkg(List(forcepkg, cyclicpackage))
        Pkg(List(forcepkg, cyclicpackage, a))
        
        

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant