Improve source gen error messages and added diagnostics

microsoft · Oct 17, 2023 · 2153ef3 · 2153ef3
1 parent d5b15dd
commit 2153ef3
Show file tree

Hide file tree

Showing 6 changed files with 72 additions and 34 deletions.
diff --git a/TestSourceGen/Program.cs b/TestSourceGen/Program.cs
@@ -1,9 +1,10 @@
 // See https://aka.ms/new-console-template for more information
 
+using Microsoft.CodeAnalysis;
 using Microsoft.Health.Fhir.SourceGenerator;
 using Microsoft.Health.Fhir.SourceGenerator.Parsing;
 
-var resourceClass = new ResourcePartialClass(typeof(Program).Namespace!, "Patient", "Patient.StructureDefinition.json", Array.Empty<string>());
+var resourceClass = new ResourcePartialClass(Location.None, typeof(Program).Namespace!, "Patient", "Patient.StructureDefinition.json", Array.Empty<string>());
 
 var emitter = new Emitter(resourceClass, diag => Console.Error.WriteLine(diag.GetMessage()));
 

diff --git a/src/Microsoft.Health.Fhir.SourceGenerator/DiagnosticDescriptors.cs b/src/Microsoft.Health.Fhir.SourceGenerator/DiagnosticDescriptors.cs
@@ -43,4 +43,20 @@ internal static class DiagnosticDescriptors
         "FhirSourceGenerator",
         DiagnosticSeverity.Info,
         true);
+
+    public static readonly DiagnosticDescriptor ResourceNameMismatch = new(
+        "FHIRGEN006",
+        "FHIR Resource name mismatch",
+        "StructureDefinition with name '{0}' mismatches with the partial class '{1}'.",
+        "FhirSourceGenerator",
+        DiagnosticSeverity.Error,
+        true);
+
+    public static readonly DiagnosticDescriptor UnableToResolveFilePath = new(
+        "FHIRGEN007",
+        "FHIR Resource file not found",
+        "Unable to find FHIR resource file at path '{0}'.",
+        "FhirSourceGenerator",
+        DiagnosticSeverity.Error,
+        true);
 }
diff --git a/src/Microsoft.Health.Fhir.SourceGenerator/Emitter.cs b/src/Microsoft.Health.Fhir.SourceGenerator/Emitter.cs
@@ -17,14 +17,14 @@ internal class Emitter
     private readonly IFhirConverter _fhirConverter = new FromFhirExpando();
     private readonly FhirVersionInfo _fhirInfo = new FhirVersionInfo(FhirPackageCommon.FhirSequenceEnum.R4B);
     private readonly ResourcePartialClass _resource;
-    private readonly Action<Diagnostic> _reportError;
+    private readonly Action<Diagnostic> _report;
 
     public Emitter(
         ResourcePartialClass resource,
-        Action<Diagnostic> reportError)
+        Action<Diagnostic> report)
     {
         _resource = resource;
-        _reportError = reportError;
+        _report = report;
     }
 
     public string? Emit()
@@ -64,11 +64,11 @@ static bool TryGetFhirTerminology(FhirVersionInfo m, string key, out object? val
         }
         catch (ReflectionTypeLoadException rex)
         {
-            _reportError(Diagnostic.Create(DiagnosticDescriptors.TypeLoaderException, Location.None, rex.LoaderExceptions));
+            _report(Diagnostic.Create(DiagnosticDescriptors.TypeLoaderException, Location.None, rex.LoaderExceptions));
         }
         catch (Exception ex)
         {
-            _reportError(Diagnostic.Create(DiagnosticDescriptors.UnhandledException, Location.None, ex));
+            _report(Diagnostic.Create(DiagnosticDescriptors.UnhandledException, Location.None, ex));
         }
 
         return null;
@@ -89,11 +89,11 @@ private void ProcessTerminologyArtifacts<TModel>(
             var model = ProcessFile(path, fhirInfo, _fhirConverter, modelCollectionLocator, out var fileName, out var canonical, out var artifactClass);
             if (model == null || (artifactClass != FhirArtifactClassEnum.CodeSystem && artifactClass != FhirArtifactClassEnum.ValueSet))
             {
-                _reportError(Diagnostic.Create(DiagnosticDescriptors.FailedArtifactDef, Location.None, path, "CodeSystem or ValueSet"));
+                _report(Diagnostic.Create(DiagnosticDescriptors.FailedArtifactDef, Location.None, path, "CodeSystem or ValueSet"));
                 continue;
             }
 
-            _reportError(Diagnostic.Create(DiagnosticDescriptors.ProcessSuccess, Location.None, path, canonical, artifactClass, fhirInfo.Resources.Count));
+            _report(Diagnostic.Create(DiagnosticDescriptors.ProcessSuccess, Location.None, path, canonical, artifactClass, fhirInfo.Resources.Count));
         }
     }
 
@@ -104,11 +104,16 @@ private void ProcessTerminologyArtifacts<TModel>(
         var complex = ProcessFile(structureDef, fhirInfo, _fhirConverter, m => m.Resources, out var id, out var canonical, out var artifactClass);
         if (complex == null || artifactClass != FhirArtifactClassEnum.Resource)
         {
-            _reportError(Diagnostic.Create(DiagnosticDescriptors.FailedArtifactDef, Location.None, structureDef, FhirArtifactClassEnum.Resource));
+            _report(Diagnostic.Create(DiagnosticDescriptors.FailedArtifactDef, Location.None, structureDef, FhirArtifactClassEnum.Resource));
+            return null;
+        }
+        else if (complex.Name != _resource.Name)
+        {
+            _report(Diagnostic.Create(DiagnosticDescriptors.ResourceNameMismatch, _resource.Location, complex.Name, _resource.Name));
             return null;
         }
 
-        _reportError(Diagnostic.Create(DiagnosticDescriptors.ProcessSuccess, Location.None, structureDef, canonical, artifactClass, fhirInfo.Resources.Count));
+        _report(Diagnostic.Create(DiagnosticDescriptors.ProcessSuccess, Location.None, structureDef, canonical, artifactClass, fhirInfo.Resources.Count));
 
         return GenerateFhirResourceSource(fhirInfo, complex, structureDef);
 
@@ -124,7 +129,7 @@ private void ProcessTerminologyArtifacts<TModel>(
 
         if (memoryStream.Length == 0)
         {
-            _reportError(Diagnostic.Create(DiagnosticDescriptors.FailedToGenerate, Location.None, originalFilePath));
+            _report(Diagnostic.Create(DiagnosticDescriptors.FailedToGenerate, Location.None, originalFilePath));
             return null;
         }
 

diff --git a/src/Microsoft.Health.Fhir.SourceGenerator/Parsing/Parser.cs b/src/Microsoft.Health.Fhir.SourceGenerator/Parsing/Parser.cs
@@ -58,6 +58,7 @@ internal IReadOnlyList<ResourcePartialClass> GetResourcePartialClasses(IEnumerab
 
                 string? structureDefPath = null;
                 string[] terminologies = Array.Empty<string>();
+                Location? location = null;
 
                 foreach (AttributeListSyntax mal in classDec.AttributeLists)
                 {
@@ -103,8 +104,8 @@ internal IReadOnlyList<ResourcePartialClass> GetResourcePartialClasses(IEnumerab
                                 switch (items.Length)
                                 {
                                     case 1:
-                                        // GenerateServiceRegistration(string configurationSection)
-                                        structureDefPath = (string)GetItem(items[0])!;
+                                        var value = GetItem(items[0]) as string;
+                                        structureDefPath = ResolvePath(value);
                                         break;
 
                                     default:
@@ -132,7 +133,13 @@ internal IReadOnlyList<ResourcePartialClass> GetResourcePartialClasses(IEnumerab
                                         {
                                             case "TerminologyResources":
                                                 var values = (ImmutableArray<TypedConstant>)GetItem(value)!;
-                                                terminologies = values.Select(x => x.Value?.ToString()).Where(x => !string.IsNullOrEmpty(x)).ToArray()!;
+                                                var testValues = values.Select(x => x.Value?.ToString()).Where(x => !string.IsNullOrEmpty(x)).ToArray()!;
+                                                terminologies = new string[testValues.Length];
+                                                for (int i = 0; i < testValues.Length; i++)
+                                                {
+                                                    terminologies[i] = ResolvePath(testValues[i])!;
+                                                }
+
                                                 break;
                                         }
                                     }
@@ -145,7 +152,31 @@ internal IReadOnlyList<ResourcePartialClass> GetResourcePartialClasses(IEnumerab
                                 break;
                             }
 
+                            location = ma.GetLocation();
                             static object? GetItem(TypedConstant arg) => arg.Kind == TypedConstantKind.Array ? arg.Values : arg.Value;
+
+                            string? ResolvePath(string? path)
+                            {
+                                if (path == null)
+                                {
+                                    return null;
+                                }
+
+                                path = Path.Combine(path.Split('/', '\\'));
+                                string found = _context.AdditionalFiles.Where(x => x.Path.EndsWith(path)).Select(x => x.Path).FirstOrDefault();
+                                if (found == null && projectDir != null)
+                                {
+                                    found = Path.Combine(projectDir, path);
+                                }
+
+                                if (!File.Exists(found))
+                                {
+                                    hasMisconfiguredInput = true;
+                                    _reportDiagnostic(Diagnostic.Create(DiagnosticDescriptors.UnableToResolveFilePath, ma.GetLocation(), found));
+                                }
+
+                                return found;
+                            }
                         }
                     }
                 }
@@ -183,21 +214,7 @@ potentialNamespaceParent is not NamespaceDeclarationSyntax &&
                         }
                     }
 
-                    result.Add(new ResourcePartialClass(nspace, classDecSymbol.Name, ResolvePath(structureDefPath), terminologies.Select(ResolvePath).ToArray()));
-
-                    string ResolvePath(string path)
-                    {
-                        path = Path.Combine(path.Split('/', '\\'));
-                        var found = _context.AdditionalFiles.Where(x => x.Path.EndsWith(path)).Select(x => x.Path).FirstOrDefault();
-                        if (found == null && projectDir != null)
-                        {
-                            found = Path.Combine(projectDir, path);
-                        }
-
-                        Debug.Assert(File.Exists(found), $"File {found} does not exist");
-
-                        return found;
-                    }
+                    result.Add(new ResourcePartialClass(location, nspace, classDecSymbol.Name, structureDefPath, terminologies));
                 }
             }
         }

diff --git a/src/Microsoft.Health.Fhir.SourceGenerator/Parsing/ResourcePartialClass.cs b/src/Microsoft.Health.Fhir.SourceGenerator/Parsing/ResourcePartialClass.cs
@@ -1,3 +1,5 @@
-namespace Microsoft.Health.Fhir.SourceGenerator.Parsing;
+using Microsoft.CodeAnalysis;
 
-internal record class ResourcePartialClass(string Namespace, string Name, string StructureDefinitionPath, string[] TerminologyResourcePaths);
+namespace Microsoft.Health.Fhir.SourceGenerator.Parsing;
+
+internal record class ResourcePartialClass(Location Location, string Namespace, string Name, string StructureDefinitionPath, string[] TerminologyResourcePaths);
diff --git a/src/Microsoft.Health.Fhir.SpecManager/Language/CSharpFirely2IG.cs b/src/Microsoft.Health.Fhir.SpecManager/Language/CSharpFirely2IG.cs
@@ -3444,9 +3444,6 @@ private void WriteGenerationComment(ExportStreamWriter writer = null)
 
             writer.WriteLineIndented("// <auto-generated/>");
             writer.WriteLineIndented($"// Contents of: {_info.PackageName} version: {_info.VersionString}");
-#if !DEBUG
-            writer.WriteLineIndented($"// Generated by {_headerUserName} on {_headerGenerationDateTime}");
-#endif
             if ((_options.ExportList != null) && _options.ExportList.Any())
             {
                 string restrictions = string.Join("|", _options.ExportList);