Fix bug handling cyclic diagnostics. (#31960)

packages/flutter/lib/src/foundation/diagnostics.dart

@@ -2486,7 +2486,7 @@ class DiagnosticsProperty<T> extends DiagnosticsNode {
       json['exception'] = exception.toString();
     json['propertyType'] = propertyType.toString();
     json['defaultLevel'] = describeEnum(_defaultLevel);
-    if (T is Diagnosticable || T is DiagnosticsNode)
+    if (value is Diagnosticable || value is DiagnosticsNode)
       json['isDiagnosticableValue'] = true;
     return json;
   }

packages/flutter/lib/src/widgets/widget_inspector.dart

@@ -1487,17 +1487,32 @@ mixin WidgetInspectorService {
     }
 
     if (config.includeProperties) {
-      List<DiagnosticsNode> properties = node.getProperties();
-      if (properties.isEmpty && value is Diagnosticable) {
-        properties = value.toDiagnosticsNode().getProperties();
+      final List<DiagnosticsNode> properties = node.getProperties();
+      if (properties.isEmpty && node is DiagnosticsProperty
+          && config.expandPropertyValues && value is Diagnosticable) {
+        // Special case to expand property values.
+        json['properties'] = _nodesToJson(
+          value.toDiagnosticsNode().getProperties().where(
+            (DiagnosticsNode node) => !node.isFiltered(DiagnosticLevel.info),
+          ),
+          _SerializeConfig(groupName: config.groupName,
+            subtreeDepth: 0,
+            expandPropertyValues: false,
+          ),
+          parent: node,
+        );
+      } else {
+        json['properties'] = _nodesToJson(
+          properties.where(
+                (DiagnosticsNode node) => !node.isFiltered(
+                createdByLocalProject ? DiagnosticLevel.fine : DiagnosticLevel
+                    .info),
+          ),
+          _SerializeConfig.merge(
+              config, subtreeDepth: math.max(0, config.subtreeDepth - 1)),
+          parent: node,
+        );
       }
-      json['properties'] = _nodesToJson(
-        properties.where(
-          (DiagnosticsNode node) => !node.isFiltered(createdByLocalProject ? DiagnosticLevel.fine : DiagnosticLevel.info),
-        ),
-        _SerializeConfig.merge(config, subtreeDepth: math.max(0, config.subtreeDepth - 1)),
-        parent: node,
-      );
     }
 
     if (node is DiagnosticsProperty) {
@@ -1515,18 +1530,6 @@ mixin WidgetInspectorService {
           'codePoint': value.codePoint,
         };
       }
-      if (config.expandPropertyValues && value is Diagnosticable) {
-        json['properties'] = _nodesToJson(
-          value.toDiagnosticsNode().getProperties().where(
-                (DiagnosticsNode node) => !node.isFiltered(DiagnosticLevel.info),
-          ),
-          _SerializeConfig(groupName: config.groupName,
-              subtreeDepth: 0,
-              expandPropertyValues: false,
-          ),
-          parent: node,
-        );
-      }
     }
     return json;
   }

packages/flutter/test/widgets/widget_inspector_test.dart

@@ -8,6 +8,7 @@ import 'dart:io' show Platform;
 import 'dart:math';
 import 'dart:ui' as ui;
 
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter/rendering.dart';
 import 'package:flutter/widgets.dart';
@@ -97,6 +98,39 @@ class _ClockTextState extends State<ClockText> {
 // End of block of code where widget creation location line numbers and
 // columns will impact whether tests pass.
 
+// Class to enable building trees of nodes with cycles between properties of
+// nodes and the properties of those properties.
+// This exposed a bug in code serializing DiagnosticsNode objects that did not
+// handle these sorts of cycles robustly.
+class CyclicDiagnostic extends DiagnosticableTree {
+  CyclicDiagnostic(this.name);
+
+  // Field used to create cyclic relationships.
+  CyclicDiagnostic related;
+  final List<DiagnosticsNode> children = <DiagnosticsNode>[];
+
+  final String name;
+
+  @override
+  String toStringShort() => '$runtimeType-$name';
+
+  // We have to override toString to avoid the toString call itself triggering a
+  // stack overflow.
+  @override
+  String toString({ DiagnosticLevel minLevel = DiagnosticLevel.debug }) {
+    return toStringShort();
+  }
+
+  @override
+  void debugFillProperties(DiagnosticPropertiesBuilder properties) {
+    super.debugFillProperties(properties);
+    properties.add(DiagnosticsProperty<CyclicDiagnostic>('related', related));
+  }
+
+  @override
+  List<DiagnosticsNode> debugDescribeChildren() => children;
+}
+
 class _CreationLocation {
   const _CreationLocation({
     @required this.file,
@@ -1145,6 +1179,40 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService {
       }
     });
 
+    testWidgets('cyclic diagnostics regression test', (WidgetTester tester) async {
+      const String group = 'test-group';
+      final CyclicDiagnostic a = CyclicDiagnostic('a');
+      final CyclicDiagnostic b = CyclicDiagnostic('b');
+      a.related = b;
+      a.children.add(b.toDiagnosticsNode());
+      b.related = a;
+
+      final DiagnosticsNode diagnostic = a.toDiagnosticsNode();
+      final String id = service.toId(diagnostic, group);
+      final Map<String, Object> subtreeJson = await service.testExtension('getDetailsSubtree', <String, String>{'arg': id, 'objectGroup': group});
+      expect(subtreeJson['objectId'], equals(id));
+      expect(subtreeJson.containsKey('children'), isTrue);
+      final List<Object> propertiesJson = subtreeJson['properties'];
+      expect(propertiesJson.length, equals(1));
+      final Map<String, Object> relatedProperty = propertiesJson.first;
+      expect(relatedProperty['name'], equals('related'));
+      expect(relatedProperty['description'], equals('CyclicDiagnostic-b'));
+      expect(relatedProperty.containsKey('isDiagnosticableValue'), isTrue);
+      expect(relatedProperty.containsKey('children'), isFalse);
+      expect(relatedProperty.containsKey('properties'), isTrue);
+      final List<Object> relatedWidgetProperties = relatedProperty['properties'];
+      expect(relatedWidgetProperties.length, equals(1));
+      final Map<String, Object> nestedRelatedProperty = relatedWidgetProperties.first;
+      expect(nestedRelatedProperty['name'], equals('related'));
+      // Make sure we do not include properties or children for diagnostic a
+      // which we already included as the root node as that would indicate a
+      // cycle.
+      expect(nestedRelatedProperty['description'], equals('CyclicDiagnostic-a'));
+      expect(nestedRelatedProperty.containsKey('isDiagnosticableValue'), isTrue);
+      expect(nestedRelatedProperty.containsKey('properties'), isFalse);
+      expect(nestedRelatedProperty.containsKey('children'), isFalse);
+    });
+
     testWidgets('ext.flutter.inspector.getRootWidgetSummaryTree', (WidgetTester tester) async {
       const String group = 'test-group';
 
@@ -1515,7 +1583,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService {
       _CreationLocation location = knownLocations[id];
       expect(location.file, equals(file));
       // ClockText widget.
-      expect(location.line, equals(50));
+      expect(location.line, equals(51));
       expect(location.column, equals(9));
       expect(count, equals(1));
 
@@ -1524,7 +1592,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService {
       location = knownLocations[id];
       expect(location.file, equals(file));
       // Text widget in _ClockTextState build method.
-      expect(location.line, equals(88));
+      expect(location.line, equals(89));
       expect(location.column, equals(12));
       expect(count, equals(1));
 
@@ -1549,7 +1617,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService {
       location = knownLocations[id];
       expect(location.file, equals(file));
       // ClockText widget.
-      expect(location.line, equals(50));
+      expect(location.line, equals(51));
       expect(location.column, equals(9));
       expect(count, equals(3)); // 3 clock widget instances rebuilt.
 
@@ -1558,7 +1626,7 @@ class TestWidgetInspectorService extends Object with WidgetInspectorService {
       location = knownLocations[id];
       expect(location.file, equals(file));
       // Text widget in _ClockTextState build method.
-      expect(location.line, equals(88));
+      expect(location.line, equals(89));
       expect(location.column, equals(12));
       expect(count, equals(3)); // 3 clock widget instances rebuilt.