Properly use @see
drumm - September 25, 2007 - 23:50
| Project: | Coder |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | stella |
| Status: | needs review |
Description
I spent some time improving API module's @see parsing. There are a few places where @see is used incorrectly in core.
Example:
* @see some_function(), some-template.tpl.php
Notes:
- @see should always be at the beginning of a line, never inline in other comments.
- Always follow function names with ().
- Keep the references on the same line.
- Separate multiple references with ', '
- No trailing punctuation, this is not a sentence.

#1
Then I guess this should be added to Coder's review rules...
#2
I'd like to add this to coder's comment review. I spent 10 minutes trying to write the rule for this, but since drumm is being very specific here, there appear to actually be several rules. If one of you wants to write the regex, I'd be happy to make sure that the coder rules syntax is correct. What coder needs, is the regex of non-conforming code. See http://drupal.org/node/144172 for help on writing rules.
#3
Hi,
I've added in @see usage rules/checks to the 5.x and 6.x versions of the coder module. It ensures that the following conditions are met:
Please download the dev version and test it.
The only condition not checked is "Keep the references on the same line.". If someone wants to provide me a rule that does this, I'd be happy to try it out. See http://drupal.org/node/144172 for help on writing rules.
Cheers,
Stella
#4
Stella, did you forget to attach the patch?
#5
You can try the attached patch or the latest 5.x-2.x-dev version.
Cheers,
Stella
#6
#7
Coder is now identifying quite a few @see problems, many of which may be false positives. I think that
@see template.tpl.phpis valid.#8
If that is the case, then requirements above are incorrect. Drumm - can you clarify the requirements for us?
Cheers,
Stella
#9
#10
All valid:
<?php/**
* @see module_function()
* @see module.file.inc
* @see module_function(), module_invoke(), module.file.inc, file.inc
*/
$foo = bar();
// @see module_function()
$foo->bar = baz();
?>
All invalid:
<?php/**
* This is special; @see module_function()
* @see module_function
* @see module.file.inc,
* module_function()
* @see module_function() module_invoke() module.file.inc file.inc
* @see module_function().
* @see module_function(), module_invoke(), module.file.inc, file.inc.
*/
$foo = bar();
// This is special; @see module_function()
$foo->bar = baz();
?>
Dunno, whether @see in single-line comments are actually parsed, but they are perfectly valid, too, so the same rules should apply.
#11
#12
Please try out the attached patch. The previous one has already been checked into CVS, so this one has an additional check for:
@see filename.incIt will just accept filenames that end in
.phpor.inc, so as to distinguish it from a normal string or a function name without a trailing (). If this is incorrect, please let me know.Cheers,
Stella
#13
Thanks heaps!
I like @see
I generally use it to (at least) refer back to the hook_***() definition when implimenting, or to the parent routine when making a trivial helper func.
Sometimes to link related form API funcs - (formname, formname_validate, formname_submit, formname_theme)
Often when I steal code from core to over-ride in my themes (over-ride of @see theme_menu_item() )
Any other suggestions folk think is good practice?
#14
@stella: A developer may reference to any file, including:
<?php/**
* @see README.txt
* @see example.module
* @see mytemplate.tpl.php
* @see myinclude.inc
* @see example.ini
*/
?>
In general, function names cannot contain dots, but filenames usually have at least one.
#15
Ok, thanks for the clarification. The attached patch assumes that if it doesn't have a . then it's a filename, otherwise it's a function and must be followed by ().
However I've just thought of another case. Is the following a valid scenario (ignore spacing)?
/*** @see myexample.php, some_function()
*
*/
Cheers,
Stella
#16
Yes, multiple references on one line are valid, and, listing a file in front of a function is valid, too.
Thoughts:
- There should be exactly one space behind @see (so
\s*should simply be)- Since multiple values are allowed, we might need a look-behind to force matching of a dot for filenames, because the dot in [a-zA-Z_\.] is optional and thus, would also match a function name.
'^\@see (([a-zA-Z_]+\(\)|[a-zA-Z_\.\-]+\.[a-zA-Z])(, )?)+'I think the first condition
[a-zA-Z_]+\(\)matches a valid function name.#17
Try the attached patch. I made a couple of changes.
I'd appreciate if you could test the patch again.
Cheers,
Stella
#18
Sorry, no review this time. Just wanted to leave a quick note that URLs should be valid for @see, too. A common practice for referencing to d.o issues, PHP bugs, and other pages is
<?php// Force all links to go to a single domain for SEO.
// @see http://drupal.org/node/195366
?>
#19
Ok will look at adding support for this.
#20
Ok try the attached patch, hopefully this one will catch all scenarios!
Cheers,
Stella