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...

CommentFileSizeAuthor
#5 coder_release.txt1.54 KBSusurrus
coder_release.inc1.37 KBSusurrus

Comments

Susurrus’s picture

Title: New extension to check code before release » Extension to check code before release
Status: Active » Needs review

Forgot to say there's a patch attached to this one.

Susurrus’s picture

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.

douggreen’s picture

Status: Needs review » Needs work

I don't think that we can universally do some of these things.

  1. The check for print_r will also need to check for the second argument, i.e., print_r($a, TRUE) is probably legitimate code.
  2. I think your check is simply looking for any semi-colon that in a comment. This will definitely result in many false positives. Please try to come up with a better check. Test running your review on all of core to see how it does.
douggreen’s picture

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.

Susurrus’s picture

StatusFileSize
new1.54 KB

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.

Susurrus’s picture

Status: Needs work » Needs review
douggreen’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev
Assigned: Unassigned » douggreen

While pruning the issue queue, I ran across this old issue, which may still be aprapros.

douggreen’s picture

Status: Needs review » Closed (fixed)

Thanks, committed! I tried it and liked what it displayed. Let's see what other people say once they start using it :)

http://drupalcode.org/project/coder.git/commit/6d610d5