Extension to check code before release
Susurrus - January 4, 2008 - 17:31
| Project: | Coder |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
Description
Code development often results in extra code that shouldn't be included for releases such as debugging statements built into PHP (print_r, var_dump) or debugging statements by using another debugging API (devel.module functions). Dead code sometimes crops up also in comments and clutters the code and reducing performance (since PHP needs to parse the files).
This checker I've written checks for the issues previously stated and is named "release." I'm just attaching the file since I'm not sure how to create a patch using WinMerge with no initial file...
| Attachment | Size |
|---|---|
| coder_release.inc | 1.37 KB |
#1
Forgot to say there's a patch attached to this one.
#2
Code is written with #206352: Refactor loading of checking extensions in mind, though the change is trivial to make: just change so the return value is an indexed array with 'release' as the only key pointing to the current return value.
#3
I don't think that we can universally do some of these things.
print_r($a, TRUE)is probably legitimate code.#4
Also, while I like the idea behind #206352 and may incorporate it, I think that it's better style to submit your patch for the current version (and to submit patches that include only one fix at a time). In this case, submitting the entire include file is perfectly acceptable. Please see creating patches.
#5
Okay, here's a revised version of the checker. It is a little more robust, though it still needs a little work. It now returns its rules as if #206352 isn't applied. Just wanted to show you some more progress.
Actually, the code check is pretty robust for code in documentation. This is because I check the last few non-whitespace characters to see if it matches any of ");", "),", "}". The only problem I've had is with "}" as there're a few lines with "{}" at the end for some reason that shouldn't be flagged. I should change this check to see if there's a ")" before the "}" I think, and then it'd be fine.
There are few false positives in comments that I've found for all installed and core, though this doesn't include code within @code tags. I'd actually like to extend coder.module to find @code tags and enter another state within them that can be checked by a rule. All code in core documentation is contained within this code, and should be within it for other modules, so code in comments that is supposed to be documentation is flagged, though the description should be changed to warn the user about @code tag usage.
#6