Page MenuHomePhabricator
Differential D10686

Add a linter rule to detect the use of blacklisted functions
ClosedPublic
Actions

Authored by joshuaspence on Oct 12 2014, 11:25 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 20, 7:35 PM
Unknown Object (File)
Tue, Nov 19, 10:39 PM
Unknown Object (File)
Fri, Nov 15, 2:09 AM
Unknown Object (File)
Sun, Nov 10, 2:02 PM
Unknown Object (File)
Wed, Nov 6, 8:58 AM
Unknown Object (File)
Oct 19 2024, 1:11 AM
Unknown Object (File)
Oct 11 2024, 5:41 PM
Unknown Object (File)
Oct 10 2024, 2:25 PM
View All Files
Subscribers
epriestley
Korvin

Details

Reviewers
epriestley
Group Reviewers
Blessed Reviewers
Commits
rARCd477df00eb5b: Add a linter rule to detect the use of blacklisted functions
Summary

As mentioned in the Phabricator PHP coding standards, the eval function should be avoided. There is some good discussion on StackOverflow as well.

Having said that, instead of hardcoding eval(), I have generalised this enough to allow a set of "blacklisted" functions to be defined with xhpast.blacklisted.function in the .arclint file.

Test Plan

Added a test case.

Diff Detail

Repository
rARC Arcanist
Branch
master
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 2808
Build 2812: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

joshuaspence updated this revision to Diff 25654.Oct 12 2014, 11:25 AM
joshuaspence retitled this revision from to Add a linter rule to detect the use of `eval`.
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 TranscriptOct 12 2014, 11:25 AM
Herald added subscribers: epriestley, Korvin. · View Herald Transcript
Harbormaster completed remote builds in B2808: Diff 25654.Oct 12 2014, 11:26 AM
joshuaspence added a comment.Jan 3 2015, 4:20 AM

bump

epriestley edited edge metadata.Jan 3 2015, 12:31 PM

Maybe we should implement this as xhpast.function-blacklist or similar (like phutil-xhpast.deprecated.functions), which would take a map from function name to explanation? That would be more flexible, at least. Not sure if there are other functions anyone might want to blacklist, though.

joshuaspence added a comment.Jan 4 2015, 11:17 PM

Seems reasonable.

joshuaspence retitled this revision from Add a linter rule to detect the use of `eval` to Add a linter rule to detect the use of blacklisted functions.Jan 4 2015, 11:18 PM
joshuaspence updated this object.
joshuaspence edited edge metadata.
joshuaspence updated this revision to Diff 26920.Jan 4 2015, 11:20 PM

Generalised to allow for cusotm blacklisted functions

joshuaspence updated this revision to Diff 26924.Jan 4 2015, 11:24 PM
joshuaspence added a child revision: D11213: Blacklist the use of `eval()`.
joshuaspence mentioned this in D11213: Blacklist the use of `eval()`.
joshuaspence added a child revision: D11214: Blacklist the use of `eval()`.
joshuaspence mentioned this in D11214: Blacklist the use of `eval()`.

Blacklist eval()

epriestley accepted this revision.Jan 4 2015, 11:30 PM
epriestley edited edge metadata.

Might be YAGNI but it's like 10 extra lines so this seems generally reasonable.

This revision is now accepted and ready to land.Jan 4 2015, 11:30 PM
Closed by commit rARCd477df00eb5b: Add a linter rule to detect the use of blacklisted functions (authored by joshuaspence, committed by joshuaspence). · Explain WhyJan 4 2015, 11:52 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
lint/
linter/
24 lines
__tests__/
xhpast/
4 lines

Diff 25654

View Options

src/lint/linter/ArcanistXHPASTLinter.php

Loading...
View Options

src/lint/linter/__tests__/xhpast/eval.lint-test

Loading...
Phabricator · Built by Phacility