Fix issues with source code HTML of fields and accessors (#2401)

* fix(fields,accessors): escape source code HTML

* test: added tests

* fix(model_node): add semicolon for fields

* test: added tests

* fix(model_node): less hacky way to solve the issue

* test: added a test

* code-style: fix comment style

* refactor(model_node): rename variable for clarity

Author: CaptainIRS

lib/src/model/accessor.dart

@@ -5,6 +5,7 @@
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/member.dart' show Member;
 import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/render/source_code_renderer.dart';
 import 'package:dartdoc/src/utils.dart';
 import 'package:dartdoc/src/warnings.dart';
 
@@ -23,6 +24,9 @@ class Accessor extends ModelElement implements EnclosedElement {
 
   bool get isSynthetic => element.isSynthetic;
 
+  SourceCodeRenderer get _sourceCodeRenderer =>
+      packageGraph.rendererFactory.sourceCodeRenderer;
+
   GetterSetterCombo _definingCombo;
   // The [enclosingCombo] where this element was defined.
   GetterSetterCombo get definingCombo {
@@ -40,8 +44,8 @@ class Accessor extends ModelElement implements EnclosedElement {
   String get sourceCode {
     if (_sourceCode == null) {
       if (isSynthetic) {
-        _sourceCode =
-            packageGraph.getModelNodeFor(definingCombo.element).sourceCode;
+        _sourceCode = _sourceCodeRenderer.renderSourceCode(
+            packageGraph.getModelNodeFor(definingCombo.element).sourceCode);
       } else {
         _sourceCode = super.sourceCode;
       }

lib/src/model/field.dart

@@ -6,6 +6,7 @@ import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/src/dart/element/element.dart';
 import 'package:dartdoc/src/element_type.dart';
 import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/render/source_code_renderer.dart';
 
 class Field extends ModelElement
     with GetterSetterCombo, ContainerMember, Inheritable
@@ -160,6 +161,9 @@ class Field extends ModelElement
   @override
   String get fileName => '${isConst ? '$name-constant' : name}.$fileType';
 
+  SourceCodeRenderer get _sourceCodeRenderer =>
+      packageGraph.rendererFactory.sourceCodeRenderer;
+
   String _sourceCode;
 
   @override
@@ -171,6 +175,7 @@ class Field extends ModelElement
       var setterSourceCode = setter?.sourceCode ?? '';
       var buffer = StringBuffer();
       if (fieldSourceCode.isNotEmpty) {
+        fieldSourceCode = _sourceCodeRenderer.renderSourceCode(fieldSourceCode);
         buffer.write(fieldSourceCode);
       }
       if (buffer.isNotEmpty) buffer.write('\n\n');

lib/src/model/model_node.dart

@@ -25,12 +25,10 @@ class ModelNode {
   final Element element;
   final ResourceProvider resourceProvider;
 
-  final int _sourceOffset;
-  final int _sourceEnd;
+  final AstNode _sourceNode;
 
   ModelNode(AstNode sourceNode, this.element, this.resourceProvider)
-      : _sourceOffset = sourceNode?.offset,
-        _sourceEnd = sourceNode?.end,
+      : _sourceNode = sourceNode,
         commentRefs = _commentRefsFor(sourceNode);
 
   static List<ModelCommentReference> _commentRefsFor(AstNode node) {
@@ -47,11 +45,25 @@ class ModelNode {
 
   String get sourceCode {
     if (_sourceCode == null) {
-      if (_sourceOffset != null) {
+      if (_sourceNode?.offset != null) {
+        var enclosingSourceNode = _sourceNode;
+
+        /// Get a node higher up the syntax tree that includes the semicolon.
+        /// In this case, it is either a [FieldDeclaration] or
+        /// [TopLevelVariableDeclaration]. (#2401)
+        if (_sourceNode is VariableDeclaration) {
+          enclosingSourceNode = _sourceNode.parent.parent;
+          assert(enclosingSourceNode is FieldDeclaration ||
+              enclosingSourceNode is TopLevelVariableDeclaration);
+        }
+
+        var sourceEnd = enclosingSourceNode.end;
+        var sourceOffset = enclosingSourceNode.offset;
+
         var contents =
             model_utils.getFileContentsFor(element, resourceProvider);
         // Find the start of the line, so that we can line up all the indents.
-        var i = _sourceOffset;
+        var i = sourceOffset;
         while (i > 0) {
           i -= 1;
           if (contents[i] == '\n' || contents[i] == '\r') {
@@ -61,8 +73,8 @@ class ModelNode {
         }
 
         // Trim the common indent from the source snippet.
-        var start = _sourceOffset - (_sourceOffset - i);
-        var source = contents.substring(start, _sourceEnd);
+        var start = sourceOffset - (sourceOffset - i);
+        var source = contents.substring(start, sourceEnd);
 
         source = model_utils.stripIndentFromSource(source);
         source = model_utils.stripDartdocCommentsFromSource(source);

test/end2end/model_test.dart

@@ -3764,6 +3764,85 @@ String topLevelFunction(int param1, bool param2, Cool coolBeans,
     });
   });
 
+  group('Source Code HTML', () {
+    Class EscapableProperties;
+    Field implicitGetterExplicitSetter, explicitGetterImplicitSetter;
+    Field explicitGetterSetter, explicitGetterSetterForInheriting;
+    Field finalProperty, simpleProperty, forInheriting;
+    Field ensureWholeDeclarationIsVisible;
+
+    setUpAll(() {
+      EscapableProperties = fakeLibrary.classes
+          .firstWhere((c) => c.name == 'HtmlEscapableProperties');
+      implicitGetterExplicitSetter = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'implicitGetterExplicitSetter');
+      explicitGetterImplicitSetter = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'explicitGetterImplicitSetter');
+      explicitGetterSetter = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'explicitGetterSetter');
+      finalProperty = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'finalProperty');
+      simpleProperty = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'simpleProperty');
+      forInheriting = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'forInheriting');
+      explicitGetterSetterForInheriting = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'explicitGetterSetterForInheriting');
+      ensureWholeDeclarationIsVisible = EscapableProperties.allModelElements
+          .firstWhere((e) => e.name == 'ensureWholeDeclarationIsVisible');
+    });
+
+    test('Normal property fields are escaped', () {
+      expect(finalProperty.sourceCode, contains('<int>'));
+      expect(simpleProperty.sourceCode, contains('<int>'));
+      expect(forInheriting.sourceCode, contains('<int>'));
+    });
+
+    test('Explicit accessors are escaped', () {
+      expect(explicitGetterSetter.getter.sourceCode, contains('<int>'));
+      expect(explicitGetterSetter.setter.sourceCode, contains('<int>'));
+    });
+
+    test('Implicit accessors are escaped', () {
+      expect(implicitGetterExplicitSetter.getter.sourceCode,
+          contains('<int>'));
+      expect(implicitGetterExplicitSetter.setter.sourceCode,
+          contains('<int>'));
+      expect(explicitGetterImplicitSetter.getter.sourceCode,
+          contains('<int>'));
+      expect(explicitGetterImplicitSetter.setter.sourceCode,
+          contains('<int>'));
+      expect(explicitGetterSetterForInheriting.getter.sourceCode,
+          contains('<int>'));
+      expect(explicitGetterSetterForInheriting.setter.sourceCode,
+          contains('<int>'));
+    });
+
+    test('Property fields are terminated with semicolon', () {
+      expect(finalProperty.sourceCode.trim(), endsWith('List<int>();'));
+      expect(simpleProperty.sourceCode.trim(), endsWith('List<int>();'));
+      expect(forInheriting.sourceCode.trim(), endsWith('forInheriting;'));
+    });
+
+    test('Arrow accessors are terminated with semicolon', () {
+      expect(explicitGetterImplicitSetter.getter.sourceCode.trim(),
+          endsWith('List<int>();'));
+      expect(explicitGetterSetter.getter.sourceCode.trim(),
+          endsWith('List<int>();'));
+    });
+
+    test('Traditional accessors are not terminated with semicolon', () {
+      expect(implicitGetterExplicitSetter.setter.sourceCode.trim(),
+          endsWith('\{\}'));
+      expect(explicitGetterSetter.setter.sourceCode.trim(), endsWith('\{\}'));
+    });
+
+    test('Whole declaration is visible when declaration spans many lines', () {
+      expect(ensureWholeDeclarationIsVisible.sourceCode,
+          contains('List<int> '));
+    });
+  });
+
   group('Sorting by name', () {
     // Order by uppercased lexical ordering for non-digits,
     // lexicographical ordering of embedded digit sequences.

testing/test_package/lib/fake.dart

@@ -564,6 +564,51 @@ class ClassWithUnusualProperties extends ImplicitProperties {
   }
 }
 
+class HtmlEscapableImplicitProperties {
+  /// Docs for implicitGetterExplicitSetter from HtmlEscapableImplicitProperties.
+  List<int> implicitGetterExplicitSetter;
+
+  /// Docs for explicitGetterImplicitSetter from HtmlEscapableImplicitProperties.
+  List<int> explicitGetterImplicitSetter;
+
+  /// A simple property to inherit.
+  List<int> forInheriting;
+
+  /// Explicit getter for inheriting.
+  List<int> get explicitGetterSetterForInheriting => [1, 2];
+
+  /// Explicit setter for inheriting.
+  set explicitGetterSetterForInheriting(List<int> foo) {}
+}
+
+class HtmlEscapableProperties extends HtmlEscapableImplicitProperties {
+  @override
+
+  /// Docs for setter of implicitGetterExplicitSetter.
+  set implicitGetterExplicitSetter(List<int> x) {}
+
+  @override
+
+  /// Getter doc for explicitGetterImplicitSetter
+  List<int> get explicitGetterImplicitSetter => List<int>();
+
+  /// Getter doc for explicitGetterSetter.
+  List<int> get explicitGetterSetter => List<int>();
+
+  /// Setter doc for explicitGetterSetter.
+  set explicitGetterSetter(List<int> aList) {}
+
+  /// A final property.
+  final List<int> finalProperty = List<int>();
+
+  /// A simple property.
+  List<int> simpleProperty = List<int>();
+
+  /// A long multiple variable declaration.
+  List<int> iAmALongLongLongLongLongLongLongLongLongName,
+      ensureWholeDeclarationIsVisible;
+}
+
 /// This is a very long line spread
 /// across... wait for it... two physical lines.
 ///