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

improve --files CLI UX #378

Merged
merged 1 commit into from Nov 12, 2023
Merged

improve --files CLI UX #378

merged 1 commit into from Nov 12, 2023

Conversation

bjaglin
Copy link
Collaborator

@bjaglin bjaglin commented Nov 12, 2023

Fixes scalacenter/scalafix#1874

To avoid getting false positive errors (missing SemanticDB in presence of semantic rules), scalafix executions with --files should not aggregate as they are by nature specific to a given project / Configuration.

  • prevent usage of scalafixAll --files
  • validate --files values against unmanagedSourceDirectories
    • allow precise completions
    • avoid aggregated executions: since requested files are not valid outside the project on which the task input was crafted, sbt simply ignores the runs

@@ -127,13 +127,6 @@ object ScalafixPlugin extends AutoPlugin {
Invisible
)

private val scalafixCompletions: SettingKey[ScalafixCompletions] =
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

inlining this saves some setting keys (sbt-scalafix adds a lot...)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This caused a regression, see #405

@@ -1,2 +1,2 @@
rules = [DisableSyntax]
rules = [DisableSyntax, RemoveUnused]
Copy link
Collaborator Author

@bjaglin bjaglin Nov 12, 2023

Choose a reason for hiding this comment

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

added a semantic rule to trigger the "semanticdb not found" error from the original bug report

Comment on lines +21 to +22
> scalafix -f=src/main/scala/OK.scala
Copy link
Collaborator Author

@bjaglin bjaglin Nov 12, 2023

Choose a reason for hiding this comment

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

implemented in this PR, thanks to the isAllowedPrefix check on aggregated projects which makes sbt skip aggregated executions silently

Comment on lines +31 to +38
-> ok_only / scalafixAll -f=ok_only/src/main/scala/OK.scala

# Targeting valid files from a different project fails
-> ok_only / scalafix -f=ok_and_ko/src/main/scala/OK.scala

# Targeting valid files from a different configuration fails
-> ok_only / Test / scalafix -f=ok_only/src/main/scala/OK.scala
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

implemented in this PR

loadedRules = () => scalafixInterfaceProvider.value().availableRules(),
terminalWidth = Some(JLineAccess.terminalWidth),
allowedTargetFilesPrefixes =
(scalafix / unmanagedSourceDirectories).value.map(_.toPath)
Copy link
Collaborator Author

@bjaglin bjaglin Nov 12, 2023

Choose a reason for hiding this comment

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

we don't rely on unmanagedSources here as

  • we want to be able to target directories as well
  • it's a task, not available at settings evaluation time anyway

To avoid getting false positive errors (missing SemanticDB in presence
of semantic rules), scalafix executions with `--files` should not
aggregate as they are by nature specific to a given project /
Configuration.

- prevent usage of scalafixAll --files
- validate --files values against unmanagedSourceDirectories
  - allow accurate completions
  - avoid aggregated executions: since requested files are not valid
    outside the project on which the task input was crafted, sbt simply
    ignores the runs
@bjaglin bjaglin marked this pull request as ready for review November 12, 2023 17:23
@bjaglin bjaglin merged commit f27ea8b into scalacenter:main Nov 12, 2023
5 checks passed
@bjaglin bjaglin mentioned this pull request Nov 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant