file_get_contents() false positive warning

[mr-macedawg]

The upgrade tool detects that the php function file_get_contents() is a part of the asset-storage probably since it starts with "file"

"file_get_contents(): Use new asset abstraction (https://docs.silverstripe.org/en/4/changelogs/4.0.0#asset-storage)"

I use file_get_content() for retrieving content from a JSON file.

[robbieaverill]

To clarify, you're saying that the upgrader tool warns you for using file_get_contents() in your project .code, and that it's assuming you're using it to access SilverStripe assets and thus suggesting you use the asset abstraction API?

If that's the case then there are two ways we could perceive this:

1. It's a good thing, because it'd catch the 80% use case. It's a warning, so it can safely be ignored.
2. It's a bad thing because it's wrong.

I'd like to think that we should be aiming to fix this. It's probably too draconian to assume that all file_get_contents() calls are for loading assets.

[maxime-rainville]

That's coming from this bit in framework's .upgrade.yml file.

  functions:
    'file_get_contents()':
      message: 'Use new asset abstraction'
      url: 'https://docs.silverstripe.org/en/4/changelogs/4.0.0#asset-storage'
    'file_put_contents()':
      message: 'Use new asset abstraction'
      url: 'https://docs.silverstripe.org/en/4/changelogs/4.0.0#asset-storage'

https://github.com/silverstripe/silverstripe-framework/blob/9ceef59e1b48ae2588e681853dfa696478de71ad/.upgrade.yml#L1315-L1321

A workaround, is too suppress the warning with a @skipUpgrade comment. Obviously that's going to be painful if you are making extensive use of a command the upgrader doesn't like.

I guess we could add some sort of way to suppress a specific warning project wide.

[dhensby]

This sounds like a feature working as expected. It's there to provide guidance not be 100% accurate about your usage...