Skip to content

Commit

Permalink
Fix: False Positive "include_file_not_found"
Browse files Browse the repository at this point in the history
See #59888 for an explanation of
the bug this commit fixes. Incidentally, the bug prevents a
"recursive_include_file" error from being reported when a file includes
itself with a quoted "include" field value. This fix happens to also
correct that so it is no longer the case.

Introduce code to handle quotations delimiting an "include" field value.
The code removes the quotes before the URI resolution is attempted. This
will prevent quotations from being interpreted as a part of the URI to
resolve. Also, add tests to verify these code changes fix the bug.

[email protected], [email protected], [email protected]

Bug: #59888
Change-Id: Idc921652bf356889142c6e38b0cdec5e66825d36
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/405180
Auto-Submit: Rohit Saily <[email protected]>
Commit-Queue: Keerti Parthasarathy <[email protected]>
Reviewed-by: Brian Wilkerson <[email protected]>
Reviewed-by: Keerti Parthasarathy <[email protected]>
  • Loading branch information
RohitSaily authored and Commit Queue committed Jan 22, 2025
1 parent 5ec1002 commit cc5612e
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 1 deletion.
8 changes: 8 additions & 0 deletions pkg/analyzer/lib/src/task/options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,14 @@ List<AnalysisError> analyzeAnalysisOptions(
var includeSpan = includeNode.span;
initialIncludeSpan ??= includeSpan;
var includeUri = includeSpan.text;
var (first, last) = (
includeUri.codeUnits.firstOrNull,
includeUri.codeUnits.lastOrNull);
if ((first == 0x0022 || first == 0x0027) && first == last) {
// The URI begins and ends with either a double quote or single quote
// i.e. the value of the "include" field is quoted.
includeUri = includeUri.substring(1, includeUri.length - 1);
}

var includedSource = sourceFactory.resolveUri(source, includeUri);
if (includedSource == initialSource) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,46 @@ main() {

@reflectiveTest
class IncludeFileNotFoundTest extends AbstractAnalysisOptionsTest {
void test_notFound() {
void test_notFound_existent_doubleQuoted() {
assertErrorsInCode('''
include: "./analysis_options.yaml"
''', [
error(AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, 9, 25)
]);
}

void test_notFound_existent_notQuoted() {
assertErrorsInCode('''
include: ./analysis_options.yaml
''', [
error(AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, 9, 23)
]);
}

void test_notFound_existent_singleQuoted() {
assertErrorsInCode('''
include: './analysis_options.yaml'
''', [
error(AnalysisOptionsWarningCode.RECURSIVE_INCLUDE_FILE, 9, 25)
]);
}

void test_notFound_nonexistent_doubleQuoted() {
assertErrorsInCode('''
# We don't depend on pedantic, but we should consider adding it.
include: "package:pedantic/analysis_options.yaml"
''', [
error(
AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND,
74,
40,
text: "The include file 'package:pedantic/analysis_options.yaml'"
" in '${convertPath('/analysis_options.yaml')}' can't be found when analyzing '/'.",
)
]);
}

void test_notFound_nonexistent_notQuoted() {
assertErrorsInCode('''
# We don't depend on pedantic, but we should consider adding it.
include: package:pedantic/analysis_options.yaml
Expand All @@ -29,4 +68,19 @@ include: package:pedantic/analysis_options.yaml
)
]);
}

void test_notFound_nonexistent_singleQuoted() {
assertErrorsInCode('''
# We don't depend on pedantic, but we should consider adding it.
include: 'package:pedantic/analysis_options.yaml'
''', [
error(
AnalysisOptionsWarningCode.INCLUDE_FILE_NOT_FOUND,
74,
40,
text: "The include file 'package:pedantic/analysis_options.yaml'"
" in '${convertPath('/analysis_options.yaml')}' can't be found when analyzing '/'.",
)
]);
}
}

0 comments on commit cc5612e

Please sign in to comment.