Page MenuHomePhabricator

Modernize `ArcanistScalaSBTLinter`.
ClosedPublic

Authored by joshuaspence on May 13 2014, 1:29 PM.
Tags
None
Referenced Files
F14395289: D9097.id21767.diff
Sun, Dec 22, 4:01 AM
Unknown Object (File)
Fri, Dec 20, 7:21 PM
Unknown Object (File)
Tue, Dec 17, 10:12 AM
Unknown Object (File)
Mon, Dec 16, 5:51 PM
Unknown Object (File)
Mon, Dec 9, 9:04 AM
Unknown Object (File)
Sat, Dec 7, 11:04 AM
Unknown Object (File)
Thu, Dec 5, 5:21 PM
Unknown Object (File)
Thu, Dec 5, 5:21 PM

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Maniphest Tasks
Restricted Maniphest Task
Commits
rARC6c1bae437e30: Modernize `ArcanistScalaSBTLinter`.
Summary

Ref T2039. Convert the ArcanistScalaSBTLinter into an ArcanistExternalLinter and make it compatible with .arclint.

Test Plan

I can't really test this because I don't have any Scala projects and we don't have any test cases.

Diff Detail

Repository
rARC Arcanist
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

joshuaspence retitled this revision from to Modernize `ArcanistScalaSBTLinter`..
joshuaspence updated this object.
joshuaspence edited the test plan for this revision. (Show Details)
joshuaspence added a reviewer: epriestley.
joshuaspence added a task: Restricted Maniphest Task.
epriestley added a subscriber: codeblock.

@codeblock, not sure if you're still around but I think you wrote the original version of this? This diff looks OK to me but I don't have this stuff installed either and I'm not sure I can figure out how to test it properly.

I'll see if I can set up a basic Scala project to test these changes.

Sounds good. If you don't get to it in the near future or it's a pain I'm happy to just pull this.

joshuaspence edited edge metadata.

Updated after testing with a sample SBT project.

OK so there are a few things that are really wrong with this "linter":

  1. It's not really a linter, it's ultimately calling sbt compile and raising any warnings/errors to ArcanistScalaSBTLinter. This makes it very slow.
  2. AFAIK, sbt compile can't be run on a single file (which makes sense). This means that (at least in its current form) if I have two files, good.scala and bad.scala then arc lint -- good.scala will raise errors in bad.scala.
  3. I have no idea why the canRun logic exists. Whilst some projects may have those directories/files, it doesn't appear to be a prerequisite. I was able to use sbt without either of those files (with a simple "Hello World" project).

IMO, this linter should be deprecated. It appears that there are true Scala linters out there (such as Scalastyle and I would personally prefer to support a true linter than a compiler.

epriestley edited edge metadata.

IMO, this linter should be deprecated. It appears that there are true Scala linters out there (such as Scalastyle and I would personally prefer to support a true linter than a compiler.

I agree; I'll just pull this for now since it improves things, but I'll accept a diff to mark it deprecated or remove it if you want. Otherwise, we can deprecate/remove it when we eventually introduce bindings for a more linty Scala linter, like Scalastyle.

This revision is now accepted and ready to land.May 18 2014, 5:49 PM
epriestley updated this revision to Diff 21813.

Closed by commit rARC6c1bae437e30 (authored by @joshuaspence, committed by @epriestley).