Bug 145954 - Web Inspector: Improve some cases of "Object?" Type Annotations
Summary: Web Inspector: Improve some cases of "Object?" Type Annotations
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: DoNotImportToRadar
Depends on:
Blocks:
 
Reported: 2015-06-12 19:33 PDT by Joseph Pecoraro
Modified: 2015-06-15 12:17 PDT (History)
9 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (1.89 KB, patch)
2015-06-12 19:38 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-06-12 19:33:07 PDT
* SUMMARY
Improve some cases of "Object?" Type Annotations.

If a type is (Object | NullOrUndefined) and no other primitives just show an "Object?" annotation instead of a nice "LeastCommonAncestor?" annotation.

* TEST
<script>
class Person {};

function personOrNullAndUndefined(person) {} // Person?
personOrNullAndUndefined(new Person);
personOrNullAndUndefined(null);
personOrNullAndUndefined(undefined);

function personOrStringOrNullAndUndefined(thing) {} // Object?
personOrStringOrNullAndUndefined(new Person);
personOrStringOrNullAndUndefined(null);
personOrStringOrNullAndUndefined(undefined);
personOrStringOrNullAndUndefined("string");

function personOrBooleanOrNullAndUndefined(thing) {} // (many)
personOrStringOrNullAndUndefined(new Person);
personOrStringOrNullAndUndefined(null);
personOrStringOrNullAndUndefined(undefined);
personOrStringOrNullAndUndefined(false);
</script>
Comment 1 Joseph Pecoraro 2015-06-12 19:35:45 PDT
> function personOrBooleanOrNullAndUndefined(thing) {} // (many)
> personOrStringOrNullAndUndefined(new Person);
> personOrStringOrNullAndUndefined(null);
> personOrStringOrNullAndUndefined(undefined);
> personOrStringOrNullAndUndefined(false);

Err, these should obviously be calling the right function. heh
Comment 2 Joseph Pecoraro 2015-06-12 19:38:08 PDT
Created attachment 254848 [details]
[PATCH] Proposed Fix
Comment 3 Joseph Pecoraro 2015-06-12 19:39:57 PDT
Noticed this by looking at PropertyPath's constructor. Parameters with:

    "PropertyPath | Undefined" showed "Object?" expected "PropertyPath?"
    "RemoteObject | Null"      showed "Object?" expected "RemoteObject?"

After the path this looks as expected.
Comment 4 Saam Barati 2015-06-12 23:12:16 PDT
Looks good to me. 
I'd r+ if I could.
Comment 5 Saam Barati 2015-06-12 23:13:56 PDT
Comment on attachment 254848 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=254848&action=review

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:135
> +        if (this._typeDescription.leastCommonAncestor) {

This is a good catch.

> Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:138
>              if (typeSet.isContainedIn(WebInspector.TypeSet.TypeBit.Object | WebInspector.TypeSet.NullOrUndefinedTypeBits))

This branch doesn't even make sense given the above branch above will preclude things that primitive types from coming through.
Comment 6 Saam Barati 2015-06-12 23:14:37 PDT
(In reply to comment #5)
> Comment on attachment 254848 [details]
> [PATCH] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=254848&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:135
> > +        if (this._typeDescription.leastCommonAncestor) {
> 
> This is a good catch.
> 
> > Source/WebInspectorUI/UserInterface/Views/TypeTokenView.js:138
> >              if (typeSet.isContainedIn(WebInspector.TypeSet.TypeBit.Object | WebInspector.TypeSet.NullOrUndefinedTypeBits))
> 
> This branch doesn't even make sense given the above branch above will
> preclude things that primitive types from coming through.
Err: This should be past tense. It didn't make sense before. It makes sense now.
Comment 7 WebKit Commit Bot 2015-06-15 12:17:30 PDT
Comment on attachment 254848 [details]
[PATCH] Proposed Fix

Clearing flags on attachment: 254848

Committed r185562: <http://trac.webkit.org/changeset/185562>
Comment 8 WebKit Commit Bot 2015-06-15 12:17:35 PDT
All reviewed patches have been landed.  Closing bug.