From 91d009664b088fc6a752cf249392a9af42376669 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 1 May 2011 11:43:12 -0700 Subject: [PATCH] Update standards documentation Summary: Move PHP standards from libphutil to Phabricator. Publish general standards draft. Test Plan: Generated, read documentation. Reviewed By: jungejason Reviewers: aran, tuomaspelkonen, jungejason CC: aran, jungejason Differential Revision: 207 --- .divinerconfig | 1 + .gitignore | 1 + src/docs/general_coding_standards.diviner | 121 ++++++++++++++++ src/docs/php_coding_standards.diviner | 164 ++++++++++++++++++++++ src/docs/upgrade_schema.diviner | 2 +- 5 files changed, 288 insertions(+), 1 deletion(-) create mode 100644 src/docs/general_coding_standards.diviner create mode 100644 src/docs/php_coding_standards.diviner diff --git a/.divinerconfig b/.divinerconfig index b5c38d4aed..377b7b565c 100644 --- a/.divinerconfig +++ b/.divinerconfig @@ -5,6 +5,7 @@ "intro" : "Introduction", "install" : "Installing", "config" : "Configuration", + "contrib" : "Contributing", "userguide" : "Application User Guides", "developer" : "Phabricator Developer Guides", "differential" : "Differential (Code Review)", diff --git a/.gitignore b/.gitignore index 07a2f0da43..6f2a14d2b2 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,4 @@ /docs/ /src/.phutil_module_cache /conf/custom/* +/.divinercache diff --git a/src/docs/general_coding_standards.diviner b/src/docs/general_coding_standards.diviner new file mode 100644 index 0000000000..f4488deabd --- /dev/null +++ b/src/docs/general_coding_standards.diviner @@ -0,0 +1,121 @@ +@title General Coding Standards +@group contrib + +This document is a general coding standard for contributing to Phabricator, Arcanist, libphutil and Diviner. + + += Overview = + +This document contains practices and guidelines which apply across languages. Contributors should follow these guidelines. These guidelines are not hard-and-fast but should be followed unless there is a compelling reason to deviated from them. + + += Code Complexity = + + - Prefer to write simple code which is easy to understand. The simplest code is not necessarily the smallest, and some changes which make code larger (such as decomposing complex expressions and choosing more descriptive names) may also make it simpler. Be willing to make size tradeoffs in favor of simplicity. + - Prefer simple methods and functions which take a small number of parameters. Avoid methods and functions which are long and complex, or take an innumerable host of parameters. When possible, decompose monolithic, complex methods into several focused, simpler ones. + - Avoid putting many ideas on a single line of code. + +For example, avoid this kind of code: + + COUNTEREXAMPLE + $category_map = array_combine( + $dates, + array_map(create_function('$z', 'return date("F Y", $z);'), $dates)); + +Expressing this complex transformation more simply produces more readable code: + + $category_map = array(); + foreach ($dates as $date) { + $category_map[$date] = date('F Y', $date); + } + +And, obviously, don't do this sort of thing: + + COUNTEREXAMPLE + if ($val = $some->complicatedConstruct() && !!~blarg_blarg_blarg() & $flags + ? HOPE_YOU_MEMORIZED == $all_the_lexical_binding_powers : <<<'Q' + ${hahaha} + Q + ); + + += Performance = + + - Prefer to write efficient code. + - Strongly prefer to drive optimization decisions with hard data. Avoid optimizing based on intuition or rumor if you can not support it with concrete measurements. + - Prefer to optimize code which is slow and runs often. Optimizing code which is fast and runs rarely is usually a waste of time, and can even be harmful if it makes that code more difficult to understand or maintain. You can determine if code is fast or slow by measuring it. + - Reject performance discussions that aren't rooted in concrete data. + +In Phabricator, you can usually use the builtin XHProf profiling to quickly gather concrete performance data. + + += Naming Things = + + - Follow language-specific conventions. + - Name things unambiguously. + - Choose descriptive names. + - Avoid nonstandard abbreviations (common abbreviations like ID, URI and HTTP are fine). + - Spell words correctly. + - Use correct grammar. + +For example, avoid these sorts of naming choices: + + COUNTEREXAMPLE + $PIE->GET_FLAVOR(); // Unconventional. + $thing->doStuff(); // Ambiguous. + $list->empty(); // Ambiguous -- is it isEmpty() or makeEmpty()? + $e = 3; // Not descriptive. + $this->updtHndlr(); // Nonstandard abbreviation. + $this->chackSpulls(); // Misspelling, ungrammatical. + +Prefer these: + + $pie->getFlavor(); // Conventional. + $pie->bake(); // Unambiguous. + $list->isEmpty(); // Unambiguous. + $list->makeEmpty(); // Unambiguous. + $edge_count = 3; // Descriptive. + $this->updateHandler(); // No nonstandard abbreviations. + $this->getID(); // Standard abbreviation. + $this->checkSpelling(); // Correct spelling and grammar. + + += Error Handling = + + - Strongly prefer to detect errors. + - Strongly prefer to fail fast and loudly. The maximum cost of script termination is known, bounded, and fairly small. The maximum cost of continuing script execution when errors have occurred is unknown and unbounded. This also makes APIs much easier to use and problems far easier to debug. + +When you ignore errors, defer error handling, or degrade the severity of errors by treating them as warnings and then dismissing them, you risk dangerous behavior which may be difficult to troubleshoot: + + COUNTEREXAMPLE + exec('echo '.$data.' > file.bak'); // Bad! + do_something_dangerous(); + + exec('echo '.$data.' > file.bak', $out, $err); // Also bad! + if ($err) { + debug_rlog("Unable to copy file!"); + } + do_something_dangerous(); + +Instead, fail loudly: + + exec('echo '.$data.' > file.bak', $out, $err); // Better + if ($err) { + throw new Exception("Unable to copy file!"); + } + do_something_dangerous(); + +But the best approach is to use or write an API which simplifies condition handling and makes it easier to get right than wrong: + + execx('echo %s > file.bak', $data); // Good + do_something_dangerous(); + + Filesystem::writeFile('file.bak', $data); // Best + do_something_dangerous(); + +See @{article:System Commands} for details on the APIs used in this example. + += Documentation, Comments and Formatting = + + - Prefer to remove code by deleting it over removing it by commenting it out. It shall live forever in source control, and can be retrieved therefrom if it is ever again called upon. + - In source code, use only ASCII printable characters plus space and linefeed. Do not use UTF-8 or other multibyte encodings. diff --git a/src/docs/php_coding_standards.diviner b/src/docs/php_coding_standards.diviner new file mode 100644 index 0000000000..fc14503024 --- /dev/null +++ b/src/docs/php_coding_standards.diviner @@ -0,0 +1,164 @@ +@title PHP Coding Standards +@group contrib + +This document describes PHP coding standards for Phabricator and related projects (like Arcanist and libphutil). + += Overview = + +This document outlines technical and style guidelines which are followed in +libphutil. Contributors should also follow these guidelines. Many of these +guidelines are automatically enforced by lint. + += Spaces, Linebreaks and Indentation = + + - Use two spaces for indentation. Don't use tab literal characters. + - Use Unix linebreaks ("\n"), not MSDOS ("\r\n") or OS9 ("\r"). + - Use K&R style braces and spacing. + - Put a space after control keywords like ##if## and ##for##. + - Put a space after commas in argument lists. + - Put a space around operators like ##=##, ##<##, etc. + - Don't put spaces after function names. + - Parentheses should hug their contents. + - Generally, prefer to wrap code at 80 columns. + += Case and Capitalization = + + - Name variables and functions using ##lowercase_with_underscores##. + - Name classes using ##UpperCamelCase##. + - Name methods and properties using ##lowerCamelCase##. + - Use uppercase for common acronyms like ID and HTML. + - Name constants using ##UPPERCASE##. + - Write ##true##, ##false## and ##null## in lowercase. + += Comments = + + - Do not use "#" (shell-style) comments. + - Prefer "//" comments inside function and method bodies. + += PHP Language Style = + + - Use "" tag. + - Prefer casts like ##(string)## to casting functions like ##strval()##. + - Prefer type checks like ##$v === null## to type functions like + ##is_null()##. + - Avoid all crazy alternate forms of language constructs like "endwhile" + and "<>". + - Always put braces around conditional and loop blocks. + += PHP Language Features = + + - Use PHP as a programming language, not a templating language. + - Avoid globals. + - Avoid extract(). + - Avoid eval(). + - Avoid variable variables. + - Prefer classes over functions. + - Prefer class constants over defines. + - Avoid naked class properties; instead, define accessors. + - Use exceptions for error conditions. + += Examples = + +**if/else:** + + if ($some_variable > 3) { + // ... + } else if ($some_variable === null) { + // ... + } else { + // ... + } + +You should always put braces around the body of an if clause, even if it is only +one line long. Note spaces around operators and after control statements. Do not +use the "endif" construct, and write "else if" as two words. + +**for:** + + for ($ii = 0; $ii < 10; $ii++) { + // ... + } + +Prefer $ii, $jj, $kk, etc., as iterators, since they're easier to pick out +visually and react better to "Find Next..." in editors. + +**foreach:** + + foreach ($map as $key => $value) { + // ... + } + +**switch:** + + switch ($value) { + case 1: + // ... + break; + case 2: + if ($flag) { + // ... + break; + } + break; + default: + // ... + break; + } + +##break## statements should be indented to block level. + +**array literals:** + + $junk = array( + 'nuts', + 'bolts', + 'refuse', + ); + +Use a trailing comma and put the closing parenthesis on a separate line so that +diffs which add elements to the array affect only one line. + +**operators:** + + $a + $b; // Put spaces around operators. + $omg.$lol; // Exception: no spaces around string concatenation. + $arr[] = $element; // Couple [] with the array when appending. + $obj = new Thing(); // Always use parens. + +**function/method calls:** + + // One line + eject($cargo); + + // Multiline + AbstractFireFactoryFactoryEngine::promulgateConflagrationInstance( + $fuel, + $ignition_source); + +**function/method definitions:** + + function example_function($base_value, $additional_value) { + return $base_value + $additional_value; + } + + class C { + public static function promulgateConflagrationInstance( + IFuel $fuel, + IgnitionSource $source) { + // ... + } + } + +**class:** + + class Dog extends Animal { + + const CIRCLES_REQUIRED_TO_LIE_DOWN = 3; + + private $favoriteFood = 'dirt'; + + public function getFavoriteFood() { + return $this->favoriteFood; + } + } + diff --git a/src/docs/upgrade_schema.diviner b/src/docs/upgrade_schema.diviner index 8da7f2c7ab..304a79f9a5 100644 --- a/src/docs/upgrade_schema.diviner +++ b/src/docs/upgrade_schema.diviner @@ -1,7 +1,7 @@ @title Upgrading Schema @group config -This document descibes how to upgrade the database schema. +This document describes how to upgrade the database schema. = Prerequisites =