WordPress / phpdoc-parser

Documentation parser powering developer.wordpress.org
https://developer.wordpress.org/reference/
239 stars 78 forks source link

Detect deprecated files #11

Closed Rarst closed 10 years ago

Rarst commented 11 years ago

There is number of deprecated files that seem to only be marked as deprecated by _deprecated_file() function call. Currently this is not reflected in parser output, so for example there is no way to know classes/functions defined in those files are deprecated (since they themselves are not marked as such).

Needs either:

JDGrimes commented 10 years ago

Maybe the best solution is to add the @deprecated tag to these files? Though that tag is only meant for structural elements, so it normally wouldn't apply to files, I guess.

GaryJones commented 10 years ago

Are all the structural elements inside "deprecated files" considered individually deprecated?

If so, then the docs for each of those elements should be updated to include @deprecated. Then the parser can just treat them as it would a deprecated element inside a "non-deprecated file".

JDGrimes commented 10 years ago

I've looked at all of the deprecated files, and it appears that any structural elements they hold are properly marked with the @deprecated tag. Third-party elements aren't marked as deprecated, but they shouldn't be, should they?

List of files that are deprecated but aren't marked with the @deprecated tag:

/wp-includes/rss-functions.php
/wp-includes/registration.php
/wp-includes/registration-functions.php
/wp-includes/class-snoopy.php

I think we should open a ticket on track to have these marked with the deprecated tag. The other deprecated files are.

DrewAPicture commented 10 years ago

@JDGrimes: Just let me know specifically what you need done and I'll move it to the top of the inline docs team's priority list.

JDGrimes commented 10 years ago

@DrewAPicture: Having second thoughts about whether anything needs to be done. I think that all of the structural elements in the deprecated files are marked as such. I think that was @Rarst's main concern here. The @deprecated tag could be added to the four files I listed above so that the parser could easily detect that they are deprecated. But as files are currently saved as a taxonomy, and therefore none of the metadata for them is imported, I don't know that it really makes any difference.

Rarst commented 10 years ago

I am not sure about "individually deprecate" as universal solution here.

See Snoopy for example. It's old and third party dependency, why mess with its inline documentation for the sake of marking it deprecated in WordPress?

JDGrimes commented 10 years ago

If we want to indicate that third-party elements are deprecated in WordPress, then we do need to have the files marked @deprecated. I think that is a better solution than making the parser look for _deprecated_file().

Rarst commented 10 years ago

Why do double effort? _deprecated calls are already in place and perform a function (throwing errors, firing hooks), so they can't be discarded in favor of inline doc. And since they are already in place why not just use that information, instead of duplicating it by adding inline doc to every instance?

JDGrimes commented 10 years ago

@DrewAPicture Does the inline docs team have an opinion on whether files that are deprecated should have the @deprecated tag? As I mentioned above, I'm not sure that it is intended to be used with files, but it is used for most deprecated files in core. So right now the usage is inconsistent.

Rarst commented 10 years ago

PSR-5 is unclear about @deprecated for files, it only mentions applying it to "structural elements", which doesn't include files. So I'd say, if without making any assumptions, files are not supposed to be using @deprecated tag.

DrewAPicture commented 10 years ago

@JDGrimes If the entire file is deprecated, its file header should hold an @deprecated marking it as such.

@Rarst I'd like it if we could blanket-mark a file deprecated by adding the tag to the file header. Is this something we can support? Also, per your earlier question about external libraries, I was always under the impression that we'd skip parsing those files altogether. If there isn't an "excluded files" list, perhaps there should be.

JDGrimes commented 10 years ago

@JDGrimes If the entire file is deprecated, its file header should hold an @deprecated marking it as such.

OK. Those four files don't.

@Rarst I'd like it if we could blanket-mark a file deprecated by adding the tag to the file header. Is this something we can support?

We already have elements inherit the @package tags from a file, so it wouldn't be hard to have them inherit the @deprecated tag as well. PSR-5 doesn't indicate that it is an inherited tag though. And I think all elements are marked with an @deprecated already, except for the external libraries.

Also, per your earlier question about external libraries, I was always under the impression that we'd skip parsing those files altogether. If there isn't an "excluded files" list, perhaps there should be.

I was thinking the same thing. I don't think that we need to provide documentation for the external libraries that WP uses. On the other hand, we will want their existence to be documented, and if they are deprecated, we'll want that recorded as well. But maybe that shouldn't be part of the parser's job?

Rarst commented 10 years ago

Also, per your earlier question about external libraries, I was always under the impression that we'd skip parsing those files altogether.

No. If it's in core it's in core. There is nothing "external" about the way WP handles libraries, essentially hardcoding them into source. If they are in — they must be in reference.

GaryJones commented 10 years ago

I've read the comments in this thread several times over trying to decide what I think would be best. For me, at the moment, I'm favouring:

There is nothing "external" about the way WP handles libraries, essentially hardcoding them into source.

Sure, but some are documented as @package External and some aren't. That might be a separate ticket to fix those.

If they are in — they must be in reference.

If everyone thought that about all code, then phpDoc tags like @internal and the de facto @ignore wouldn't exist, along with it's opposite, @api. We're not trying to recreate the source from scratch here, and there's some bits, like empty files or files that have been deprecated for (nearly) 9 major versions that don't need to be cluttering up the documentation reference. Clarity is better than the kitchen sink.

Rarst commented 10 years ago

I stick with my position — everything in is in. The prime major flaw of Codex as reference is and always was poor completeness of information.

As a developer I don't want someone else to decide what I should and should not see, especially in giant code base with long history of inline documentation issues (no offense to recent docs efforts, they are great and welcomed improvement on the situation).

This reference isn't being built at shallow APIs levels, it should be able to serve core developer/contributor as well as novice.

I suggest leaving information out is discussed as separate dedicated issue if there is interest.

GaryJones commented 10 years ago

The prime major flaw of Codex as reference is and always was poor completeness of information.

Sure, but now we've got something automated, and the docs are far more complete than they've ever been for WP. That gives us the flexibility to sensibly decide what to exclude, rather than playing catch up on what should be included. We're approaching it from a different perspective.

As a developer I don't want someone else to decide what I should and should not see

No, but as someone producing the (tool that creates the) documentation, you should be deciding, and a blanket on "everything" is only helpful to the top of the developer pyramid (and they've got source to look through...).

it should be able to serve core developer/contributor as well as novice.

If a core developer / contributor wanted to see the dirty details, then the source is always going to be better to look through than the documentation scraped from it. As such, I would expect to see DevHub as the place for novice and intermediate developers, who don't favour going through the source, to understand how WordPress works now, not how it used to work.

Does the DevHub project as a whole have a mission statement / aim / goals listed where this is / can be clarified?

Rarst commented 10 years ago

You are derailing discussion of deprecated here, as above - please open separate dedicated issue if you are interested in discussing this in detail.

GaryJones commented 10 years ago

It's all related - whether everything, including all things marked as deprecated, should be included or not. It was your comment that started the discussion btw; I tacked on a response to it on my post that outlined a suggestion for tackling the deprecated issues.

Rarst commented 10 years ago

Regardless of DevHub content goals my personal goal for Parser is to parse everything. Not necessarily achievable goal in scope needed for DevHub, but still.

We can argue that it's not a priority to be implemented as it's not needed for DevHub, but parsing deprecated, internal, and whatever else things is happening at some point.

So as I see it what-DevHub-displays is entirely separate discussion, likely not even on this issue tracker.

Scope of this ticket:

GaryJones commented 10 years ago

+1 to all of what you just said and point well-made.

My apologies for mixing up the WP-Parser project and the DevHub project.

JDGrimes commented 10 years ago

Note that it would be possible to do this by checking for the use of _deprecated_file() once #79 is complete, assuming that we store uses information for files and not just other functions (I don't know if that has been discussed).

Rarst commented 10 years ago

Hmm... We currently have file taxonomy for definitions, so we cannot reuse it for calls, taxonomies cannot do two different kinds of relationship.

If we go with post relationships it does make sense to me that files should be CPT with two relationships — what is defined inside of them and what is called inside of them.

bobbingwide commented 10 years ago

Hi, once again, I'm following this discussion while working on a parallel project, rather than contributing. Having parsed WordPress code with WP-Parser and my own solution I realised I needed to be able to parse the logic within files which wasn't part of a class, method or function. To do this I needed to create a "file" CPT. It's similar to an API ( method or function ) in that it can call functions, invoke hooks and even associate functions to hooks. The post meta fields that I have for a file are as below.

bw_register_field( "_oik_file_name", "text", "File name" , array( "#length" => 80 )); bw_register_field( "_oik_api_plugin", "noderef", "Plugin ref", array( "#type" => "oik-plugins" )); bw_register_field( "_oik_file_passes", "numeric", "Parse count", array( "#theme" => false )); bw_register_field( "_oik_file_deprecated_cb", "checkbox", "Deprecated?"); bw_register_field( "_oik_api_calls", "noderef", "Uses APIs", array( "#type" => "oik_api", "#multiple" => true, "#optional" => true, '#theme' => false )); bw_register_field( "_oik_api_hooks", "noderef", "Uses hooks", array( "#type" => "oik_hook", "#multiple" => true, "#optional" => true, '#theme' => false ));

As you can see I have a deprecated checkbox. BUT setting its value is much lower on my priority list than parsing the code to populate the relationships to APIs and hooks.

The next stage will be to handle file relationships - allowing navigation through require(), include() and helper functions that themselves perform file loading.

To achieve this I believe I will have to develop some rule based logic, similar to that in makepot. These rules could then be extended to fire action hooks for _deprecated related functions.

Rarst commented 10 years ago

Docs team decided not to use @deprecated for files, so we are definitely back to detecting _deprecated_file() calls.

Rarst commented 10 years ago

Fixed by #133.