Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yaml::parse #66

Open
smatejic opened this issue Oct 26, 2015 · 6 comments
Open

Yaml::parse #66

smatejic opened this issue Oct 26, 2015 · 6 comments

Comments

@smatejic
Copy link

Using Yaml::parse is also deprecated and it needs rule

$config = Yaml::parse(file_get_contents($someConfigFile));

The original function
/**
* Parses YAML into a PHP array.
*
* The parse method, when supplied with a YAML stream (string or file),
* will do its best to convert YAML in a file into a PHP array.
*
* Usage:
*
* $array = Yaml::parse(file_get_contents('config.yml'));
* print_r($array);
*

*
* As this method accepts both plain strings and file names as an input,
* you must validate the input before calling this method. Passing a file
* as an input is a deprecated feature and will be removed in 3.0.
*
* Note: the ability to pass file names to the Yaml::parse method is deprecated since version 2.2 and will be removed in 3.0. Pass the YAML contents of the file instead.
*
* @param string $input Path to a YAML file or a string containing YAML
* @param bool $exceptionOnInvalidType True if an exception must be thrown on invalid types false otherwise
* @param bool $objectSupport True if object support is enabled, false otherwise
* @param bool $objectForMap True if maps should return a stdClass instead of array()
*
* @return array The YAML converted to a PHP array
*
* @throws ParseException If the YAML is not valid
*
* @api
*/
public static function parse($input, $exceptionOnInvalidType = false, $objectSupport = false, $objectForMap = false)
{
// if input is a file, process it
$file = '';
if (strpos($input, "\n") === false && is_file($input)) {
@trigger_error('The ability to pass file names to the '.METHOD.' method is deprecated since version 2.2 and will be removed in 3.0. Pass the YAML contents of the file instead.', E_USER_DEPRECATED);

        if (false === is_readable($input)) {
            throw new ParseException(sprintf('Unable to parse "%s" as the file is not readable.', $input));
        }

        $file = $input;
        $input = file_get_contents($file);
    }

    $yaml = new Parser();

    try {
        return $yaml->parse($input, $exceptionOnInvalidType, $objectSupport, $objectForMap);
    } catch (ParseException $e) {
        if ($file) {
            $e->setParsedFile($file);
        }

        throw $e;
    }
}

This also needs rule since deprecator does not see this.
Maybe we should make PR on Symfony with @deprecated ?

This way you can not see deprecated notice on detector

@xabbuh
Copy link
Member

xabbuh commented Oct 26, 2015

The @deprecated annotation was not added by intent in core as only parts of the method are deprecated (just the feature to be able to use a filename instead of a YAML string).

@smatejic
Copy link
Author

Yes but the new way to do that stuff suggested by docs is replacing
Symfony\Component\Yaml\Yaml;
with
Symfony\Component\Yaml\Parser;

and then replace
$yml = Yaml::parse($path);
with
$parser = new Parser();
$yml = $parser->parse(file_get_contents($path));

The way it is now, there was a merge to fix this problem by adding file_get_contents($path) but as suggested in Doc it will be removed in 3.0

 * As this method accepts both plain strings and file names as an input,
 * you must validate the input before calling this method. Passing a file
 * as an input is a deprecated feature and will be removed in 3.0.

@ghost
Copy link

ghost commented Oct 26, 2015

I think this discussion would be better held in the according repository, however we could include files which contain trigger_error and E_USER_DEPRECATED in the search for deprecations.

@xabbuh
Copy link
Member

xabbuh commented Oct 26, 2015

I still think that this isn't an issue you can resolve with static code analysis (you can hardly tell if the passed value is a filename or simply a YAML string).

@slde-flash
Copy link

i am 👎 - guessing filenames based on strings is very error prone.

@MarvinKlemp
Copy link
Contributor

IMO we could implement something like a warning, because it is like @xabbuh said impossible to know the value of variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants