inspect warnings written into code

silverstripe / silverstripe-upgrader

A tool to help upgrade your code to handle API changes in packages you used.

BSD 3-Clause "New" or "Revised" License

40 stars 21 forks source link

inspect warnings written into code #176

Open sunnysideup opened 5 years ago

sunnysideup commented 5 years ago

I was wondering if it would be possible to have the inspect warnings written into the code...

e.g.

BEFORE

Session::get('foo', 'bar');

AFTER

//@todo: upgrade me ... Session can not be called using static methods in SS4, see bla.foobar.com
Session::get('foo', 'bar');

The reason for this is that they are easily missed when doing the upgrade on the command line and if you are working on the project over several days then you want an easy way to find the "todo" things.

sunnysideup commented 5 years ago

I am happy to write this code for the upgrade tool IF someone can me a few pointers on how to do this.

Alternatively, someone could perhaps explain how to get the warnings data in a parseable format so that we could run the inspect command and add the comments ourselves using the outputted data.

e.g.

[PATH], [LINE], [MESSAGE], [URL]
[PATH], [LINE], [MESSAGE], [URL]
[PATH], [LINE], [MESSAGE], [URL]
'app/src/Hello.php', 23, 'Session can not be called using static methods in SS4', 'bla.foobar.com'

maxime-rainville commented 5 years ago

I'm not sure how much value this would provide. The initial logic wasn't written with this idea in mind.

If I wanted to do this, I would probably try to add it to CodeChangeSet, this is where we track the changed code and the warnings. That's what uses to compiled the diff we show users.

If you want to have a go at creating a PR for this, maybe do a very rough proof of concept first.

sunnysideup commented 5 years ago

In my mind it adds a ton of value, for the following reasons:

in our current system, as soon as you make one change to a file, the line reference to further warnings are lost.
because you can search through the code for "UPGRADE REQUIRED", or whatever you want to use, and then you can see the whole code immediately, in all its detail. We use this system in https://github.com/sunnysideup/silverstripe-upgrade_to_silverstripe_4 and it is one of the best parts. It adds a neat comment above any line that needs changing, with the suggested change and explanation.
because saving the warnings to the actual code means they are never "overlooked" and you delete the warning from the code once completed.

Would it be possible to get the output like this: [PATH], [LINE], [MESSAGE], [URL] 'app/src/Hello.php', 23, 'Session can not be called using static methods in SS4', 'bla.foobar.com'

or perhaps have the option to save this into a file?