Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(471)

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 137553005: DevTools: [JsDocValidator] Refactor JsDoc annotation checkers (Closed)

Created:
6 years, 11 months ago by apavlov
Modified:
6 years, 10 months ago
Reviewers:
eustas, vsevik, aandrey, sergeyv
CC:
blink-reviews, caseq+blink_chromium.org, loislo+blink_chromium.org, eustas+blink_chromium.org, alph+blink_chromium.org, yurys+blink_chromium.org, lushnikov+blink_chromium.org, vsevik+blink_chromium.org, pfeldman+blink_chromium.org, paulirish+reviews_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, aandrey+blink_chromium.org
Visibility:
Public.

Description

DevTools: [JsDocValidator] Refactor JsDoc annotation checkers - Migrate to a uniform JS context-tracking mechanism - Add tests R=aandrey, eustas, sergeyv, vsevik NOTRY=true Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=166124

Patch Set 1 #

Patch Set 2 : Patch for review #

Patch Set 3 : Limit the thread count by the number of validated files #

Total comments: 18

Patch Set 4 : Code inspection nits fixed #

Patch Set 5 : Nullify owner type for nested functions #

Patch Set 6 : Improve JsDoc detection for functions #

Patch Set 7 : Rebase #

Total comments: 14

Patch Set 8 : Address comments from sergeyv #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1546 lines, -461 lines) Patch
M Source/devtools/scripts/jsdoc-validator/hashes View 1 2 3 4 5 6 7 1 chunk +2 lines, -2 lines 0 comments Download
M Source/devtools/scripts/jsdoc-validator/jsdoc-validator.jar View 1 2 3 4 5 6 7 Binary file 0 comments Download
A Source/devtools/scripts/jsdoc-validator/run_tests.py View 1 1 chunk +74 lines, -0 lines 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/DoDidVisitorAdapter.java View 1 chunk +3 lines, -1 line 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java View 1 2 3 5 chunks +5 lines, -18 lines 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/JsDocValidator.java View 1 2 3 4 5 6 7 3 chunks +37 lines, -6 lines 0 comments Download
A + Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/ValidationCheck.java View 1 2 3 2 chunks +5 lines, -8 lines 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/ValidatorContext.java View 1 2 3 4 5 6 7 2 chunks +4 lines, -1 line 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/AstUtil.java View 1 2 3 4 5 6 7 3 chunks +58 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingChecker.java View 1 2 3 4 5 6 7 1 chunk +25 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java View 1 2 3 4 5 6 7 1 chunk +58 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingValidationCheck.java View 1 2 3 4 5 6 7 1 chunk +201 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/FunctionRecord.java View 1 2 3 1 chunk +28 lines, -0 lines 0 comments Download
D Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ProtoFollowsExtendsAnnotationCheck.java View 1 chunk +0 lines, -185 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ProtoFollowsExtendsChecker.java View 1 2 3 4 5 6 7 1 chunk +126 lines, -0 lines 0 comments Download
D Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/RequiredReturnAnnotationCheck.java View 1 chunk +0 lines, -139 lines 0 comments Download
D Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/RequiredThisAnnotationCheck.java View 1 chunk +0 lines, -76 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/RequiredThisAnnotationChecker.java View 1 2 3 4 5 6 7 1 chunk +52 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ReturnAnnotationChecker.java View 1 2 3 4 5 6 7 1 chunk +145 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/TypeRecord.java View 1 2 3 4 5 6 7 1 chunk +34 lines, -0 lines 0 comments Download
M Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ValidationCheck.java View 1 1 chunk +0 lines, -25 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/tests/golden.dat View 1 chunk +165 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/tests/proto.js View 1 chunk +56 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/tests/return.js View 1 chunk +293 lines, -0 lines 0 comments Download
A Source/devtools/scripts/jsdoc-validator/tests/this.js View 1 2 3 4 5 1 chunk +175 lines, -0 lines 0 comments Download

Messages

Total messages: 13 (0 generated)
apavlov
6 years, 11 months ago (2014-01-24 15:32:27 UTC) #1
apavlov
PTAL
6 years, 11 months ago (2014-01-27 09:10:54 UTC) #2
eustas
phase 1: code inspection. https://codereview.chromium.org/137553005/diff/100001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java File Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java (right): https://codereview.chromium.org/137553005/diff/100001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java#newcode73 Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java:73: this.context = context; final? https://codereview.chromium.org/137553005/diff/100001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/JsDocValidator.java ...
6 years, 10 months ago (2014-01-28 08:47:20 UTC) #3
apavlov
https://codereview.chromium.org/137553005/diff/100001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java File Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java (right): https://codereview.chromium.org/137553005/diff/100001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java#newcode73 Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/FileCheckerCallable.java:73: this.context = context; On 2014/01/28 08:47:21, eustas wrote: > ...
6 years, 10 months ago (2014-01-28 10:40:16 UTC) #4
sergeyv
https://codereview.chromium.org/137553005/diff/200001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java File Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java (right): https://codereview.chromium.org/137553005/diff/200001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java#newcode29 Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java:29: final Deque<TypeRecord> typeRecords = new ArrayDeque<>(); you can use ...
6 years, 10 months ago (2014-01-29 09:42:58 UTC) #5
apavlov
https://codereview.chromium.org/137553005/diff/200001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java File Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java (right): https://codereview.chromium.org/137553005/diff/200001/Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java#newcode29 Source/devtools/scripts/jsdoc-validator/src/org/chromium/devtools/jsdoc/checks/ContextTrackingState.java:29: final Deque<TypeRecord> typeRecords = new ArrayDeque<>(); On 2014/01/29 09:42:58, ...
6 years, 10 months ago (2014-01-29 10:35:05 UTC) #6
apavlov
Seva, can you take a look please? None of the reviewers have lgtm'ed so far...
6 years, 10 months ago (2014-01-29 15:38:44 UTC) #7
eustas
lgtm
6 years, 10 months ago (2014-01-30 11:40:49 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/apavlov@chromium.org/137553005/220001
6 years, 10 months ago (2014-01-30 11:42:22 UTC) #9
commit-bot: I haz the power
CQ bit was unchecked on CL. Ignoring.
6 years, 10 months ago (2014-01-30 12:55:45 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/apavlov@chromium.org/137553005/220001
6 years, 10 months ago (2014-01-30 12:56:06 UTC) #11
commit-bot: I haz the power
Change committed as 166124
6 years, 10 months ago (2014-01-30 12:56:53 UTC) #12
commit-bot: I haz the power
6 years, 10 months ago (2014-01-30 12:57:07 UTC) #13
Message was sent while issue was closed. 
CQ bit was unchecked on CL. Ignoring.

Powered by Google App Engine
This is Rietveld 408576698