Paths

Table of Contentst

Differential D9089

Be more strict with the type of `.arclint` properties.
ClosedPublic
Actions

Authored by joshuaspence on May 13 2014, 8:49 AM.

Tags

None

Referenced Files

	Unknown Object (File)
	Thu, Dec 19, 6:48 PM

	Unknown Object (File)
	Mon, Dec 16, 11:07 PM

	Unknown Object (File)
	Sun, Dec 15, 7:08 AM

	Unknown Object (File)
	Wed, Dec 11, 1:22 AM

	Unknown Object (File)
	Fri, Dec 6, 5:30 AM

	Unknown Object (File)
	Thu, Dec 5, 12:05 PM

	Unknown Object (File)
	Wed, Dec 4, 9:33 PM

	Unknown Object (File)
	Wed, Dec 4, 9:33 PM

Subscribers

Details

Reviewers

Group Reviewers

Blessed Reviewers

Commits

rARCb744ed9a19ce: Be more strict with the type of `.arclint` properties.

Summary

Although this provides less context in terms of the error message (for example, Parameter has invalid type. Expected type 'optional regex|list<regex>', got type 'list<string>'.), I think that it is the right approach. I think that PhutilTypeSpec::checkMap should be improved such that additional context is provided in the exception message.

Test Plan

Ran arc lint. Modified .arclint to contain an invalid regex and ran arc lint again.

Diff Detail

Repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

joshuaspence updated this revision to Diff 21595.May 13 2014, 8:49 AM

joshuaspence retitled this revision from to Be more strict with the type of `.arclint` properties..

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 13 2014, 8:49 AM

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

Harbormaster completed remote builds in B417: Diff 21595.May 13 2014, 8:50 AM

Let's at least improve the error message on the easy one? I don't think PhutilTypeSpec can know about that.

src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
71–86	Here, at least, we should be able to catch and throw a PhutilProxyException like below...
99	(Like this.)

This revision now requires changes to proceed.May 13 2014, 8:14 PM

joshuaspence added inline comments.May 13 2014, 9:50 PM

src/lint/engine/ArcanistConfigurationDrivenLintEngine.php
71–86	Sure, but how can we know which key was invalid?

Just raising the linter key would be helpful -- "There was an error in the configuration for linter "%s": ..." or similar

Use PhutilProxyException to throw a more descriptive error message.

Harbormaster completed remote builds in B443: Diff 21641.May 13 2014, 9:58 PM

good enough for government work

This revision is now accepted and ready to land.May 13 2014, 10:17 PM

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

Revision Contents
Changeset List

Path

Size

src/

lint/

engine/

ArcanistConfigurationDrivenLintEngine.php

71 lines

Diff 21642

src/lint/engine/ArcanistConfigurationDrivenLintEngine.php

Loading...