Skip to content

Commit 9b71817

Browse files
DanTupCommit Queue
authored andcommitted
[analysis_server] Fix duplicate reporting of some LSP type parsing errors
Parse errors for spec types were being reported twice - once by the containing object (in canParse()), and once by the nested canParse() call for the nested type. This skips reporting the error for nested calls to a canParse() method, which will always report the error itself. Change-Id: I03e4a9638fd6a3fc77eac918f6fd16def93327d6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/404105 Reviewed-by: Samuel Rawlins <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]>
1 parent c36dac8 commit 9b71817

File tree

4 files changed

+24
-215
lines changed

4 files changed

+24
-215
lines changed

pkg/analysis_server/test/tool/lsp_spec/json_test.dart

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ void main() {
288288
expect(reporter.errors.first, equals('params.kind must not be null'));
289289
});
290290

291-
test('canParse records fields of the wrong type', () {
291+
test('canParse records fields of the wrong type (non-spec types)', () {
292292
var reporter = LspJsonReporter('params');
293293
expect(RenameFileOptions.canParse({'overwrite': 1}, reporter), isFalse);
294294
expect(reporter.errors, hasLength(1));
@@ -298,6 +298,20 @@ void main() {
298298
);
299299
});
300300

301+
test('canParse records fields of the wrong type (spec types)', () {
302+
var reporter = LspJsonReporter('params');
303+
expect(
304+
ClientCapabilities.canParse({'textDocument': 1}, reporter),
305+
isFalse,
306+
);
307+
expect(
308+
reporter.errors.single,
309+
equals(
310+
'params.textDocument must be of type TextDocumentClientCapabilities',
311+
),
312+
);
313+
});
314+
301315
test('canParse records nested undefined fields', () {
302316
var reporter = LspJsonReporter('params');
303317
expect(

pkg/analysis_server/tool/lsp_spec/codegen_dart.dart

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -514,8 +514,15 @@ void _writeCanParseType(
514514

515515
buffer
516516
..write(') {')
517-
..indent()
518-
..writeIndentedln('reporter.reportError($quote$failureMessage$quote);')
517+
..indent();
518+
if (!_isSpecType(type)) {
519+
// Only report an error for non-spec types, as spec types will have reported
520+
// their own error in the nested canParse() call.
521+
buffer.writeIndentedln(
522+
'reporter.reportError($quote$failureMessage$quote);',
523+
);
524+
}
525+
buffer
519526
..writeIndentedln('return false;')
520527
..outdent()
521528
..writeIndentedln('}')

third_party/pkg/language_server_protocol/lib/protocol_custom_generated.dart

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,6 @@ bool _canParseArgumentEdit(
5252
}
5353
if ((!nullCheck || value != null) &&
5454
!ArgumentEdit.canParse(value, reporter)) {
55-
reporter.reportError('must be of type ArgumentEdit');
5655
return false;
5756
}
5857
} finally {
@@ -102,7 +101,6 @@ bool _canParseElement(
102101
return false;
103102
}
104103
if ((!nullCheck || value != null) && !Element.canParse(value, reporter)) {
105-
reporter.reportError('must be of type Element');
106104
return false;
107105
}
108106
} finally {
@@ -128,7 +126,6 @@ bool _canParseErrorCodes(
128126
}
129127
if ((!nullCheck || value != null) &&
130128
!ErrorCodes.canParse(value, reporter)) {
131-
reporter.reportError('must be of type ErrorCodes');
132129
return false;
133130
}
134131
} finally {
@@ -154,7 +151,6 @@ bool _canParseFlutterOutline(
154151
}
155152
if ((!nullCheck || value != null) &&
156153
!FlutterOutline.canParse(value, reporter)) {
157-
reporter.reportError('must be of type FlutterOutline');
158154
return false;
159155
}
160156
} finally {
@@ -180,7 +176,6 @@ bool _canParseInsertTextFormat(
180176
}
181177
if ((!nullCheck || value != null) &&
182178
!InsertTextFormat.canParse(value, reporter)) {
183-
reporter.reportError('must be of type InsertTextFormat');
184179
return false;
185180
}
186181
} finally {
@@ -501,7 +496,6 @@ bool _canParseMethod(
501496
return false;
502497
}
503498
if ((!nullCheck || value != null) && !Method.canParse(value, reporter)) {
504-
reporter.reportError('must be of type Method');
505499
return false;
506500
}
507501
} finally {
@@ -526,7 +520,6 @@ bool _canParseOutline(
526520
return false;
527521
}
528522
if ((!nullCheck || value != null) && !Outline.canParse(value, reporter)) {
529-
reporter.reportError('must be of type Outline');
530523
return false;
531524
}
532525
} finally {
@@ -551,7 +544,6 @@ bool _canParsePosition(
551544
return false;
552545
}
553546
if ((!nullCheck || value != null) && !Position.canParse(value, reporter)) {
554-
reporter.reportError('must be of type Position');
555547
return false;
556548
}
557549
} finally {
@@ -576,7 +568,6 @@ bool _canParseRange(
576568
return false;
577569
}
578570
if ((!nullCheck || value != null) && !Range.canParse(value, reporter)) {
579-
reporter.reportError('must be of type Range');
580571
return false;
581572
}
582573
} finally {
@@ -602,7 +593,6 @@ bool _canParseResponseError(
602593
}
603594
if ((!nullCheck || value != null) &&
604595
!ResponseError.canParse(value, reporter)) {
605-
reporter.reportError('must be of type ResponseError');
606596
return false;
607597
}
608598
} finally {
@@ -653,7 +643,6 @@ bool _canParseTextDocumentIdentifier(
653643
}
654644
if ((!nullCheck || value != null) &&
655645
!TextDocumentIdentifier.canParse(value, reporter)) {
656-
reporter.reportError('must be of type TextDocumentIdentifier');
657646
return false;
658647
}
659648
} finally {
@@ -679,7 +668,6 @@ bool _canParseTypeHierarchyAnchor(
679668
}
680669
if ((!nullCheck || value != null) &&
681670
!TypeHierarchyAnchor.canParse(value, reporter)) {
682-
reporter.reportError('must be of type TypeHierarchyAnchor');
683671
return false;
684672
}
685673
} finally {

0 commit comments

Comments
 (0)