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


--- Comment #0 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-06-01
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


--- Comment #1 from bearophile_hugs eml.cc 2013-06-02 04:09:02 PDT ---
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



--- Comment #2 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-06-02
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



--- Comment #3 from bearophile_hugs eml.cc 2013-06-02 06:31:31 PDT ---
(In reply to comment #0)

 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



--- Comment #4 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-06-02
06:47:29 PDT ---
(In reply to comment #3)
 (In reply to comment #0)
 
 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


--- Comment #5 from Walter Bright <bugzilla digitalmars.com> 2013-06-04
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


--- Comment #6 from lt.infiltrator gmail.com 2013-06-04 22:50:44 PDT ---
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



--- Comment #7 from bearophile_hugs eml.cc 2013-06-05 05:04:07 PDT ---
(In reply to comment #5)

 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



--- Comment #8 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-06-05
19:49:54 PDT ---
(In reply to comment #5)
 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



--- Comment #9 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-08-25
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



--- Comment #10 from bearophile_hugs eml.cc 2013-08-25 09:54:14 PDT ---
(In reply to comment #9)

 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



--- Comment #11 from bearophile_hugs eml.cc 2013-08-25 10:57:20 PDT ---
(In reply to comment #9)

 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



--- Comment #12 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-08-25
18:19:43 PDT ---
(In reply to comment #11)
 (In reply to comment #9)
 
 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



--- Comment #13 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-08-25
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



--- Comment #14 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-08-25
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



--- Comment #15 from bearophile_hugs eml.cc 2013-08-25 19:12:47 PDT ---
(In reply to comment #14)

 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



--- Comment #16 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-08-27
04:38:47 PDT ---
(In reply to comment #15)
 (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



--- Comment #17 from Lionello Lunesu <lio+bugzilla lunesu.com> 2013-09-01
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



--- Comment #18 from github-bugzilla puremagic.com 2013-09-15 00:52:10 PDT ---
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
Merge pull request #2121 from lionello/bug10236

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