Add a linter rule for determining when single quotes should be used over double quotes.
ClosedPublic
Actions

Authored by joshuaspence on May 14 2014, 1:36 PM.

Details

Reviewers

epriestley

Group Reviewers

Blessed Reviewers

Commits

rARC35a26718d861: Add a linter rule for determining when single quotes should be used over double…

Summary

Personally, I am a strong fan of this rule. There is currently a similar rule provided by PHP_CodeSniffer.

Test Plan

Wrote and executed unit tests.

Diff Detail

Repository

rARC Arcanist

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

joshuaspence updated this revision to Diff 21659.May 14 2014, 1:36 PM

joshuaspence retitled this revision from to Add a linter rule for determining when single quotes should be used over double quotes..

joshuaspence updated this object.

joshuaspence edited the test plan for this revision. (Show Details)

joshuaspence added a reviewer: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptMay 14 2014, 1:36 PM

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

Harbormaster completed remote builds in B458: Diff 21659.May 14 2014, 1:37 PM

What does it do in this case?

return pht(
  "This string requires \x12\x34 double quotes, but ".
  "this string does not. Here, they are used for consistency.");

In D9117#6, @epriestley wrote:

What does it do in this case?

return pht(
  "This string requires \x12\x34 double quotes, but ".
  "this string does not. Here, they are used for consistency.");

Yeah, I hadn't considered this case. I'll need to do a little more work on this.

joshuaspence planned changes to this revision.May 14 2014, 2:23 PM

I think there's some code in the analysis of pht() and such that can merge string literals together if you fish around a bit. That use-the-less-preferred-quotes-for-consistency case is the only one I can think of where this gets the rule wrong.

Sure, I'll take a look. Thanks.

Off topic, but I have given thought to extending the ArcanistXHPASTLinter and generally making it more customisable to (possibly) allow for more widespread use. One point of concern, however, is that the source code is getting a tad large.

joshuaspence added a parent revision: D9134: Add an `selectDescendantsOfTypes` to the `AASTNode`..May 15 2014, 1:57 PM

Fixed for the specific test case that @epriestley raised.

Harbormaster completed remote builds in B484: Diff 21698.May 15 2014, 1:58 PM

I might need to comment this stuff a bit...

epriestley accepted this revision.May 17 2014, 2:25 AM

epriestley edited edge metadata.

epriestley added inline comments.

src/lint/linter/ArcanistXHPASTLinter.php
2416	I think we could remove escapes on double quotes here? But that's complicated and probably not worthwhile.
2425	I'm going to tweak this message slightly to emphasize that the motivation here is style consistency, to something like this: String does not require double quotes; for consistency, prefer single quotes. (Long ago, at Facebook, there was a wiki page which advised you to use this quote style for performance, which has no basis in reality.)