www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 10236] New: Ddoc: Warning on wrong parameter names

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236

           Summary: Ddoc: Warning on wrong parameter names
           Product: D
           Version: unspecified
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: lio+bugzilla lunesu.com



21:40:51 PDT ---
Ddoc does not complain when the parameters from "Params:" mismatch the actual
parameters used in the function declaration.

I have a patch that issues as warning in such case. An additional case would be
issuing a warning for missing parameters.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 01 2013
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236


bearophile_hugs eml.cc changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bearophile_hugs eml.cc



Such tests were discussed time ago, and to me they seem a good idea.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




06:22:52 PDT ---
https://github.com/D-Programming-Language/dmd/pull/2121

Issues warnings for both the name and the count mismatch.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236







 An additional case would be issuing a warning for missing parameters.


Another useful warning is when you document an argument that's not present in
the argument list.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




06:47:29 PDT ---



 

 An additional case would be issuing a warning for missing parameters.


 
 Another useful warning is when you document an argument that's not present in
 the argument list.


That's my first case: the parameters from "Params:" have to be present in the
actual declaration. Next, I also check the parameter count.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 02 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bugzilla digitalmars.com
            Version|unspecified                 |D2



22:27:34 PDT ---
I have mixed feelings about this. Generally, Ddoc does not emit warnings or
errors. This is deliberate, as it tries to interpret whatever random stuff the
user may write as Ddoc stuff. If it isn't valid Ddoc, Ddoc shouldn't complain.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 04 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236


lt.infiltrator gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |lt.infiltrator gmail.com



I get what you mean, Walter; but sometimes you'll make a change and forget to
update the ddoc.  Perhaps making it complain by default isn't the correct way,
and I know you hate the idea of adding more switches, but maybe have a
--check-ddoc (or I guess with a single hash for dmd) for the ability to check
your ddoc and say "hey, you forgot this; this doesn't match with your code;
etc."

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 04 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236







 I have mixed feelings about this. Generally, Ddoc does not emit warnings or
 errors. This is deliberate, as it tries to interpret whatever random stuff the
 user may write as Ddoc stuff. If it isn't valid Ddoc, Ddoc shouldn't complain.


Languages as Python have more flexibility thanks to their dynamic typing. D has
chosen to be a (mostly) statically typed language. So let's use such static
typing (and Ahead Of Time compilation) for all it's able to give.

The tests proposed here are another excellent way to improve the D
documentation using the capabilities D has, just like the documentation
unittests.

Having documentation not in sync with the actual code is _worse_ than not
having documentation. This enhancement request offers a chance to reduce such
mistakes. Documentation bugs are bugs, because they are part of the API to
interface a library with the brain of the humans that use it. Introducing quick
compiler-enforced tests to reduce the amount of such mistakes is a good idea.

Instead of a warning I'd like this to become an error, eventually :-) If the
programmer writes no documentation string, or he/she writes only a freestyle
comment, no error is generated. If he/she chooses to use the ddoc syntax to
list the arguments, that later the ddoc will pick to show them in a pretty
formatted way, they must be correct.

It's common for comments to go out of sync with the code. Just like "static
switch" this is an automatic help to keep different parts of the code in sync.

If you don't like this proposal, then please Walter write down a list of
downsides and problems that this will cause. And let's see if each one of your
points will hold.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 05 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




19:49:54 PDT ---


 I have mixed feelings about this. Generally, Ddoc does not emit warnings or
 errors. This is deliberate, as it tries to interpret whatever random stuff the
 user may write as Ddoc stuff. If it isn't valid Ddoc, Ddoc shouldn't complain.


In fact, Ddoc already has a couple of "warnings", that's why I stuck to
warnings for this issue as well. You can't expect users to use Ddoc comments
and then write non-Ddoc text. If that was the case, why do we have Ddoc
comments to begin with?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 05 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




09:08:16 PDT ---
Updated the pull request
https://github.com/D-Programming-Language/dmd/pull/2121

FWIW, this patch has found numerous DDoc mistakes in phobos and druntime! I'd
say that's proof enough.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236







 this patch has found numerous DDoc mistakes in phobos and druntime!


A different outcome would have shocked me :-)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236







 FWIW, this patch has found numerous DDoc mistakes in phobos and druntime! I'd
 say that's proof enough.


Where's a compact list of all the Phobos documentation errors found by the new
warnings?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




18:19:43 PDT ---



 

 FWIW, this patch has found numerous DDoc mistakes in phobos and druntime! I'd
 say that's proof enough.


 
 Where's a compact list of all the Phobos documentation errors found by the new
 warnings?


http://d.puremagic.com/issues/show_bug.cgi?id=10893

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




18:23:42 PDT ---
...and I filed this one for druntime:
http://d.puremagic.com/issues/show_bug.cgi?id=10894

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




18:35:13 PDT ---
While solving the warnings in druntime, there's only one instance where I
thought the current patch might need to be improved:

  /**
     * Resets this fiber so that it may be re-used.  This routine may only be
     * called for fibers that have terminated, as doing otherwise could result
     * in scope-dependent functionality that is not executed.  Stack-based
     * classes, for example, may not be cleaned up properly if a fiber is reset
     * before it has terminated.
     *
     * Params:
     *  fn = The fiber function.
     *  dg = The fiber function.
     *
     * In:
     *  This fiber must be in state TERM.
     */
    final void reset();

    /// ditto
    final void reset( void function() fn );

    /// ditto
    final void reset( void delegate() dg );

This shows three warnings for the first reset(), no warnings for the "ditto". A
quick fix would be to copy the whole ddoc for the two overloads, but perhaps
there's something smarter that can be done here?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236







 This shows three warnings for the first reset(), no warnings for the "ditto". A
 quick fix would be to copy the whole ddoc for the two overloads, but perhaps
 there's something smarter that can be done here?


Here "Params" is used in a non-standard way, it shows the merged arguments of
all the overloads of the "reset" function. This merging should be accepted by
the warning code only if dittos are used. So a possible solution is to create a
set of all the arguments of the overloads tagged with "ditto", and then verify
such set is the same as the set of arguments listed in the "Params" section.

(But usually warnings aren't 100.00% reliable. Usually there are very uncommon
cases where a warning gives false positives and false negatives. This ddoc
warning seems to be nearly perfect, but I don't expect it to be really perfect.
I think here a error rate of 0.1% or 0.01% is acceptable. Keeping warning
implementation simple is sometimes better than trying to further reduce down
that tiny percentage of errors.)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 25 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




04:38:47 PDT ---


 (But usually warnings aren't 100.00% reliable. Usually there are very uncommon
 cases where a warning gives false positives and false negatives. This ddoc
 warning seems to be nearly perfect, but I don't expect it to be really perfect.
 I think here a error rate of 0.1% or 0.01% is acceptable. Keeping warning
 implementation simple is sometimes better than trying to further reduce down
 that tiny percentage of errors.)


I agree that this case is not a priority and the patch would be acceptable with
it unsolved.

Another issue though is that I'm not checking template parameters. This is
actually a bigger issue, as it happens far more often in Phobos and druntime.
In fact, there are many occurences of both: many templates that do and many
that don't document the template parameters.

Now I wonder whether it's a good idea to use "Params:" for both compile time
and runtime parameters. In the very least the generated documentation should
mention whether it's one or the other.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 27 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




20:51:22 PDT ---
Pull request was updated to allow for template parameters:
 * parameters in the DDoc Params section must either be function parameters of
template parameters
 * all function parameters must appear in the Params section, but this is not
checked for template parameters.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 01 2013
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236




Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/9dbb905f84dfe5a240033179172ff603f2a4b741
Fix issue 10236 Ddoc: Warning on wrong parameter names/count

https://github.com/D-Programming-Language/dmd/commit/51afa9a24d94211e2e93dfd11bcb66c4d76ed96a
Fix issue 10236 test case

https://github.com/D-Programming-Language/dmd/commit/359dc6697b92ca3c7b1fc4fddc30c2b981cf56eb
Fix issue 10236: allow template parameters in Params section + test

https://github.com/D-Programming-Language/dmd/commit/151ad0104e457ed3476bd4b2b9447bb7ce710cc5


Fix issue 10236 Ddoc: Warning on wrong parameter names/count

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 15 2013
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=10236


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
           Severity|normal                      |enhancement


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 15 2013