Fix a bug in the way analysis option files are merged

bwilkerson · commit-bot@chromium.org · commit f193637d8261 · 2021-04-29T04:08:14.000Z
Fixes: https://github.com/dart-lang/linter/issues/2612 Change-Id: I1e383a040a47c75ba55b2eee787850543159232d Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/197242 Reviewed-by: Samuel Rawlins <srawlins@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com> Commit-Queue: Brian Wilkerson <brianwilkerson@google.com>
diff --git a/pkg/analyzer/lib/src/util/yaml.dart b/pkg/analyzer/lib/src/util/yaml.dart
@@ -30,7 +30,7 @@ class Merger {
   ///   * maps are merged recursively.
   ///   * if map values cannot be merged, the overriding value is taken.
   ///
-  YamlNode merge(YamlNode o1, YamlNode? o2) {
+  YamlNode merge(YamlNode o1, YamlNode o2) {
     // Handle promotion first.
     YamlMap listToMap(YamlList list) {
       Map<YamlNode, YamlNode> map =
@@ -56,7 +56,10 @@ class Merger {
       return mergeList(o1, o2);
     }
     // Default to override, unless the overriding value is `null`.
-    return o2 ?? o1;
+    if (o2 is YamlScalar && o2.value == null) {
+      return o1;
+    }
+    return o2;
   }
 
   /// Merge lists, avoiding duplicates.
@@ -75,7 +78,7 @@ class Merger {
   YamlMap mergeMap(YamlMap m1, YamlMap m2) {
     Map<YamlNode, YamlNode> merged =
         HashMap<YamlNode, YamlNode>(); // equals: _equals, hashCode: _hashCode
-    m1.nodes.forEach((k, v) {
+    m1.nodeMap.forEach((k, v) {
       merged[k] = v;
     });
     m2.nodeMap.forEach((k, v) {
diff --git a/pkg/analyzer/test/source/analysis_options_provider_test.dart b/pkg/analyzer/test/source/analysis_options_provider_test.dart
@@ -195,6 +195,31 @@ include: foo.include
     }
   }
 
+  void test_getOptions_include_emptyLints() {
+    pathTranslator.newFile('/foo.include', r'''
+linter:
+  rules:
+    - prefer_single_quotes
+''');
+    pathTranslator.newFile('/$analysisOptionsYaml', r'''
+include: foo.include
+linter:
+  rules:
+    # avoid_print: false
+''');
+    YamlMap options = _getOptions('/');
+    expect(options, hasLength(2));
+    {
+      var linter = options.valueAt('linter') as YamlMap;
+      expect(linter, hasLength(1));
+      {
+        var rules = linter.valueAt('rules') as YamlList;
+        expect(rules, hasLength(1));
+        expect(rules[0], 'prefer_single_quotes');
+      }
+    }
+  }
+
   void test_getOptions_include_missing() {
     pathTranslator.newFile('/$analysisOptionsYaml', r'''
 include: /foo.include
