Page MenuHomePhabricator

Modernize `ArcanistScalaSBTLinter`.
ClosedPublic

Authored by joshuaspence on May 13 2014, 1:29 PM.
Tags
None
Referenced Files
F13814762: D9097.id21813.diff
Thu, Sep 19, 6:54 AM
F13814760: D9097.id.diff
Thu, Sep 19, 6:54 AM
F13814523: D9097.id21767.diff
Thu, Sep 19, 5:42 AM
F13814522: D9097.id21609.diff
Thu, Sep 19, 5:42 AM
F13814520: D9097.diff
Thu, Sep 19, 5:42 AM
Unknown Object (File)
Sat, Aug 31, 11:53 AM
Unknown Object (File)
Thu, Aug 29, 12:04 AM
Unknown Object (File)
Thu, Aug 22, 6:33 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
Branch
scala
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 524
Build 524: [Placeholder Plan] Wait for 30 Seconds

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