Handle (missing) trailing slash in dartdoc

Handle (missing) trailing slash in dartdoc


BUG=2311
Committed: https://code.google.com/p/dart/source/detail?r=14536

[Johnni Winther, 2012-11-05 13:42:50]

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (left):

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart#oldc...
utils/apidoc/apidoc.dart:556: if (member is MethodMirror &&
(member.isConstructor || member.isFactory)) {
Dead code (and a resolution error - it's now called isFactoryConstructor)

[ahe, 2012-11-05 15:58:42]

LGTM

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
File sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:47: throw new ArgumentError("Library
URI must end with a /");
Library URI -> libraryRoot

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:50: throw new ArgumentError("Package
URI must end with a /");
Package URI -> packageRoot

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:39: throw new
ArgumentError("Library URI must end with a /");
Library URI -> libraryRoot

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:42: throw new
ArgumentError("Package URI must end with a /");
Package URI -> packageRoot

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (left):

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart#oldc...
utils/apidoc/apidoc.dart:556: if (member is MethodMirror &&
(member.isConstructor || member.isFactory)) {
On 2012/11/05 13:42:50, Johnni Winther wrote:
> Dead code (and a resolution error - it's now called isFactoryConstructor)

What is dead code, and what is a resolution error? Did you mean warning?

[Johnni Winther, 2012-11-05 16:23:29]

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
File sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:47: throw new ArgumentError("Library
URI must end with a /");
On 2012/11/05 15:58:42, ahe wrote:
> Library URI -> libraryRoot

Done.

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/co...
sdk/lib/_internal/compiler/compiler.dart:50: throw new ArgumentError("Package
URI must end with a /");
On 2012/11/05 15:58:42, ahe wrote:
> Package URI -> packageRoot

Done.

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/apiimpl.dart (right):

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:39: throw new
ArgumentError("Library URI must end with a /");
On 2012/11/05 15:58:42, ahe wrote:
> Library URI -> libraryRoot

Done.

https://codereview.chromium.org/11369074/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/apiimpl.dart:42: throw new
ArgumentError("Package URI must end with a /");
On 2012/11/05 15:58:42, ahe wrote:
> Package URI -> packageRoot

Done.

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (left):

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart#oldc...
utils/apidoc/apidoc.dart:556: if (member is MethodMirror &&
(member.isConstructor || member.isFactory)) {
On 2012/11/05 15:58:42, ahe wrote:
> On 2012/11/05 13:42:50, Johnni Winther wrote:
> > Dead code (and a resolution error - it's now called isFactoryConstructor)
> 
> What is dead code, and what is a resolution error? Did you mean warning?

The isFactory method has been renamed to isFactoryConstructor in a previous CL,
and yes, we're not in Java so that is just a dynamic NoSuchMethodError. 

In any case, a factory is also a constructor, so the code is dead.

[ahe, 2012-11-05 16:46:08]

Nitpicking

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart
File utils/apidoc/apidoc.dart (left):

https://codereview.chromium.org/11369074/diff/1/utils/apidoc/apidoc.dart#oldc...
utils/apidoc/apidoc.dart:556: if (member is MethodMirror &&
(member.isConstructor || member.isFactory)) {
On 2012/11/05 16:23:29, Johnni Winther wrote:
> On 2012/11/05 15:58:42, ahe wrote:
> > On 2012/11/05 13:42:50, Johnni Winther wrote:
> > > Dead code (and a resolution error - it's now called isFactoryConstructor)
> > 
> > What is dead code, and what is a resolution error? Did you mean warning?
> 
> The isFactory method has been renamed to isFactoryConstructor in a previous
CL,
> and yes, we're not in Java so that is just a dynamic NoSuchMethodError. 
> 
> In any case, a factory is also a constructor, so the code is dead.

Terminology by example:

if (false) {
  print('This is dead code');
}

if (x < 2 || x < 1) {
  print("This is not dead code, it's just a redundant test");
}

So this expression "member is MethodMirror && (member.isConstructor ||
member.isFactory)" does not contain dead code, and it throws a dynamic error if
[member] is a MethodMirror that isn't a constructor.