Check for unquoted strings that aren't defined constants

This could actually be a valid line if you had a define() for mymodule_settings_page. I think that the discussion we had once upon a time was about the strings (and defines) as it related to some other rule. However, it is a standard for defines to be upper case. So in this case, I think we could catch it with a rule that looks for 'regex' => 'drupal_get_form\([a-z_]+\s*[,\)]', 'not' => 'drupal_get_form\([a-z]+\s*\(', ... regex's are untested.

Well, the general case problem is for ALL function calls or unquoted strings anywhere unexpected to raise warnings, I don't see what's special about drupal_get_form. So that means paranoia everywhere.

So this becomes a PHP parsing thing. I guess log level notice may show some of that.
Or, what I do is use a proper IDE that does syntax highlighting and helps locate issues like that. I use Eclipse which subtly indicates when I'm using an undefined constant or variable.

@dman, good point, as it sounds like the consensus is that this is a pretty special case, I'm marking as "won't fix."

Unquoted stings are legal in PHP. Although they just get cast to literal strings ... which is almost never what you want.
But constants for example are fine.

I think that the coder module is going to have a hard time telling the difference between

print(IMAGE_ORIGINAL); // A constant defined elsewhere, a long way away.
print(IMAGE_MEDIUM);   // An unquoted string? Should it raise an error? Or was it defined somewhere already?

Or do you think it's possible to come up with an expression that can detect unexpected unquoted stings - that are NOT function names, constants, variables or PHP keywords - better than the PHP parser itself can?

print("an example 'hook' is <pre>  hook_example(arg1) </pre>"); // this line should not trigger any problem
drupal_set_message('call_me'. call_me_too); // this line is a problem
drupal_set_message('call_me'. $call_me_three); // this isn't
$output = "Incorrect strings may concievably be ". anywhere . in . the . code;
$callback_func = my_callback; // and they are not always wrong. This line may be legal.
for (){}; // All PHP constructs may look like just unquoted strings unless we keep a list.
call_func(true); // It's a lot of work for a regexp to know that this is OK
call_func(maybe); // and this is (maybe) not
define('maybe', 0.5); // and now it is. Although not exactly Drupal-compliant still.

... etc. Yikes.
:-(

It's really a PHP syntax tokenizer job. I think the parser does raise notices if the volume is turned right up.

@dman, if we wanted to work with this, I think that we could rely on the standard that defined constants are UPPERCASE.

@NancyDru, But I think that the number of use-cases to check will be very hard to do.

@dougreen, yes, if nothing else, non-upper constants are bad drupal-style anyway, and already raise warnings.

That previous thread makes sense, as using unquoted array keys is a known (although discouraged) practice.
Also it would have still worked as expected, and does come down to a style and best-practice choice. Including efficiency of course.

Unquoted strings as in the OP example are much more rare ... in many languages anyway, and are more probably just code mistakes than stylistic choice.

In that discussion many of my misgivings are also addressed.

However, there's a difference in code style (discouraging a deprecated practice) and actual syntax checking broken code.
I also think that the regexp patterns needed to assure no false positives (if even possible) would be horrible. :-/

If someone want to demo a proof-of-concept for the general case, good luck, but it scares me.

Note that having Coder find those errors is great, using E_NOTICE on your development system helps too... That is, if you at least once, run that line of code...

So I don't think it is too important that Coder successfully checks those. Instead, make sure to add to your documentation something about the E_NOTICE probably not being turned ON by default. And maybe a few other things such as the Core function that catch errors must be changed to show all the E_NOTICE errors too!

For more info, I strongly advise that you look into this one:

#761780: Plenty of E_ALL notices in modules developed on installation with Drupal tarball

And it is difficult to use E_NOTICE when you have tons and tons of modules because half of them will generate ("legal") notices!

Thank you.
Alexis

Closing old issue seems to have lost interest, these problems seem to be caught in other ways ???