Paths

Table of Contentst

Add a Scalastyle linter
Needs RevisionPublic
Actions

Authored by josh-newman on Sep 3 2014, 8:12 AM.

Details

Reviewers

joshuaspence
epriestley

Group Reviewers

Blessed Reviewers

Required Signatures

L28 Phacility Individual Contributor License Agreement

Summary

I've been using Phabricator a lot recently for Scala projects. We thought it might be useful to add linting to our code review process, and I didn't see any current Scala linters in Arcanist, so I put together a Scalastyle linter. I didn't find existing revisions or tasks related to Scala linter configuration, but if there are I'm happy to wait or contribute to that instead of pushing this change.

I'm not too familiar with Arcanist contribution processes; please let me know if I'm doing anything very wrongly!

Test Plan

I ran this linter locally.

I tried adding a unit test but I wasn't sure how to configure the paths to the Scalastyle JAR and configuration file in the test environment. Are there examples of other tests that do this?

Diff Detail

Repository

rARC Arcanist

Branch

scalastyle

Lint

Lint Passed

Unit

Test Failures

Build Status

Buildable 2417
Build 2421: [Placeholder Plan] Wait for 30 Seconds

Unit TestsFailed
View All

	Time	Test
	0 ms	testPHPLint
	0 ms	testEverythingImplemented
	0 ms	testFixLetterCase
	0 ms	testJSONLinter
	0 ms	testJscsLinter
		View Full Test Results (1 Failed · 14 Passed · 14 Skipped)

Event Timeline

josh-newman updated this revision to Diff 25036.Sep 3 2014, 8:12 AM

josh-newman retitled this revision from to Add a Scalastyle linter.

josh-newman updated this object.

josh-newman edited the test plan for this revision. (Show Details)

josh-newman added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptSep 3 2014, 8:12 AM

Herald added a required legal document: L28 Phacility Individual Contributor License Agreement. · View Herald Transcript

Herald added subscribers: Korvin, epriestley. · View Herald Transcript

Harbormaster completed remote builds in B2417: Diff 25036.Sep 3 2014, 8:13 AM

@epriestley, is there anyone else I should send this revision to for review? (I just picked your name out of the contribution guide.) Thanks!

We have about 80 revisions in the queue for review, so it may take some time. For a change to go in, we need to be able to fully test, document, and support it in the upstream, so it's not a quick process. Sorry. :(

Add check for empty stdout.

Harbormaster completed remote builds in B2586: Diff 25288.Sep 19 2014, 4:40 AM

turadg awarded a token.Sep 25 2014, 6:10 PM

turadg added a subscriber: turadg.

This linter should offer an option to use the ScalaStyle SBT plugin.

This revision now requires changes to proceed.Sep 29 2014, 2:11 PM

chad removed a reviewer: rafikk.Sep 29 2014, 2:16 PM

• hanfeisun awarded a token.Dec 23 2014, 3:19 AM

• hanfeisun added a subscriber: • hanfeisun.

Maybe Scala Abide is a better choice for scala linter because it is maintained by Scala Official Repository

Thanks for the reference, @hanfeisun. It looks like scala-abide isn't released yet, though; do you know when it'll be ready for general use?

joshuaspence added a reviewer: joshuaspence.Feb 27 2015, 5:47 AM

In D10405#115418, @josh-newman wrote:

Thanks for the reference, @hanfeisun. It looks like scala-abide isn't released yet, though; do you know when it'll be ready for general use?

https://github.com/scala/scala-abide/issues/43#issuecomment-76624724

He said there isn't an ETA yet, but not far from a 0.1 release. will have a better estimate after ScalaDays (in 2 weeks)..

Some initial feedback.

src/lint/linter/ArcanistScalastyleLinter.php
14	This should be wrapped in a call to the `pht()` function so that it is translatable.
33–35	This is the default behaviour, so you don't need to override this method explicitly.
38–45	You should be able to remove the `optional` type specifier from `getLinterConfigurationOptions()` instead.
48	Prefer `'-jar='.$this->jarPath`
49	As above.
50	Prefer `--quiet` (without `true`).
53–55	Just remove this.
114	Should be namespaced as `scalastyle.jar`
115	Why a list of strings?
120	As above.
121	Why a list of strings?

Revision Contents
Changeset List

Path

Size

src/

__phutil_library_map__.php

2 lines

lint/

linter/

ArcanistScalastyleLinter.php

172 lines

Diff 25036

View Options

src/__phutil_library_map__.php