I just found out when .info has syntax error, the core module system can still install the problem module and show no error. However, this cause problem with "Update status". For example, I made a mistake like this in my .info:

name = My Module Name
description = this string do not parse "because it has quote" in the middle

Eventhough the second line is bad, somehow module can still install. But when this .info file is checked in to contributes repo, new release version get out-of-date warning in Update status, and recommend using the previous version. I believe the problem is error in parsing the second line so the rest of the .info lines gets ignore and there is no datastamp to compare.

So it would be nice if Coder can test .info for syntax error as part of code review by running parse_ini_file() on .info file and check for 'false' return.

Comments

mattyoung’s picture

mattyoung commented

Nevermind the bit about using parse_ini_file(). I found out the info files are parsed with this:

drupal_parse_info_file()

douggreen’s picture

douggreen commented
Version: 6.x-1.0 » 7.x-2.x-dev

Bumping this to the latest version because I know that we can do better with .info files ... I think there are a few other related tickets, that need to be consolidated into one.

douggreen’s picture

douggreen commented
Assigned: Unassigned » douggreen
douggreen’s picture

douggreen commented
Status: Active » Closed (fixed)

That doesn't seem to be the problem. This string does seem to parse.

If I write a test file:

<?php
print_r(drupal_parse_info_format("name = My Module Name\ndescription = this string do not parse \"because it has quote\" in the middle"));

It prints out:

Array
(
    [name] => My Module Name
    [description] => this string do not parse "because it has quote" in the middle
)

I don't think this is a good idea, because coder can apply it's rules to older or future releases of Drupal, but relying on drupal_parse_info_format() or drupal_parse_info_file() will always tie that review to the current version of Drupal running. If we added it to the 7.x upgrade review, and then that function changes in 8.x, when you run coder 8.x using the 7.x upgrade review, you will get different results than if you ran coder 7.x with the 7.x upgrade review.

If we know of specific problems to look for, we should look for those issues. The upgrade reviews already does a bit of this. The info file was added in 6.x. The 6.x upgrade review makes sure you have an info file. In 7.x the keys "core" and "files" were added and the review checks to make sure you've added these. If there are more changes in 8.x, we'll check for those too. And if there is some generic problem that you can identify, we'll check for that too. But I am somewhat against (based on the above argument) using a Drupal core function to test itself.