Rework "Not unique exporter for exporter type" PR

src/BenchmarkDotNet/Configs/DebugConfig.cs

@@ -52,7 +52,6 @@ public override IEnumerable<Job> GetJobs()
 
     public abstract class DebugConfig : IConfig
     {
-        private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();
         public abstract IEnumerable<Job> GetJobs();
 
         public IEnumerable<IValidator> GetValidators() => Array.Empty<IValidator>();
@@ -85,7 +84,5 @@ public string ArtifactsPath
         public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => Array.Empty<BenchmarkLogicalGroupRule>();
 
         public ConfigOptions Options => ConfigOptions.KeepBenchmarkFiles | ConfigOptions.DisableOptimizationsValidator;
-
-        public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;
     }
 }

src/BenchmarkDotNet/Configs/DefaultConfig.cs

@@ -20,7 +20,6 @@ namespace BenchmarkDotNet.Configs
     public class DefaultConfig : IConfig
     {
         public static readonly IConfig Instance = new DefaultConfig();
-        private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();
 
         private DefaultConfig()
         {
@@ -68,6 +67,7 @@ public IEnumerable<IValidator> GetValidators()
             yield return GenericBenchmarksValidator.DontFailOnError;
             yield return DeferredExecutionValidator.FailOnError;
             yield return ParamsAllValuesValidator.FailOnError;
+            yield return RPlotExporterValidator.FailOnError;
         }
 
         public IOrderer Orderer => null;
@@ -93,8 +93,6 @@ public string ArtifactsPath
             }
         }
 
-        public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;
-
         public IEnumerable<Job> GetJobs() => Array.Empty<Job>();
 
         public IEnumerable<BenchmarkLogicalGroupRule> GetLogicalGroupRules() => Array.Empty<BenchmarkLogicalGroupRule>();

src/BenchmarkDotNet/Configs/IConfig.cs

@@ -52,10 +52,5 @@ public interface IConfig
         /// the auto-generated project build timeout
         /// </summary>
         TimeSpan BuildTimeout { get; }
-
-        /// <summary>
-        /// Collect any errors or warnings when composing the configuration
-        /// </summary>
-        IReadOnlyList<Conclusion> ConfigAnalysisConclusion { get; }
     }
 }

src/BenchmarkDotNet/Configs/ImmutableConfig.cs

@@ -52,8 +52,7 @@ internal ImmutableConfig(
             IOrderer orderer,
             SummaryStyle summaryStyle,
             ConfigOptions options,
-            TimeSpan buildTimeout,
-            IReadOnlyList<Conclusion> configAnalysisConclusion)
+            TimeSpan buildTimeout)
         {
             columnProviders = uniqueColumnProviders;
             loggers = uniqueLoggers;
@@ -73,7 +72,6 @@ internal ImmutableConfig(
             SummaryStyle = summaryStyle;
             Options = options;
             BuildTimeout = buildTimeout;
-            ConfigAnalysisConclusion = configAnalysisConclusion;
         }
 
         public ConfigUnionRule UnionRule { get; }
@@ -116,7 +114,5 @@ public IDiagnoser GetCompositeDiagnoser(BenchmarkCase benchmarkCase, RunMode run
 
             return diagnosersForGivenMode.Any() ? new CompositeDiagnoser(diagnosersForGivenMode) : null;
         }
-
-        public IReadOnlyList<Conclusion> ConfigAnalysisConclusion { get; private set; }
     }
-}
+}

src/BenchmarkDotNet/Configs/ImmutableConfigBuilder.cs

@@ -27,6 +27,7 @@ public static class ImmutableConfigBuilder
             ShadowCopyValidator.DontFailOnError,
             JitOptimizationsValidator.DontFailOnError,
             DeferredExecutionValidator.DontFailOnError,
+            RPlotExporterValidator.FailOnError,
             ParamsAllValuesValidator.FailOnError
         };
 
@@ -37,11 +38,10 @@ public static ImmutableConfig Create(IConfig source)
         {
             var uniqueColumnProviders = source.GetColumnProviders().Distinct().ToImmutableArray();
             var uniqueLoggers = source.GetLoggers().ToImmutableHashSet();
-            var configAnalyse = new List<Conclusion>();
 
             var uniqueHardwareCounters = source.GetHardwareCounters().ToImmutableHashSet();
             var uniqueDiagnosers = GetDiagnosers(source.GetDiagnosers(), uniqueHardwareCounters);
-            var uniqueExporters = GetExporters(source.GetExporters(), uniqueDiagnosers, configAnalyse);
+            var uniqueExporters = GetExporters(source.GetExporters(), uniqueDiagnosers);
             var uniqueAnalyzers = GetAnalysers(source.GetAnalysers(), uniqueDiagnosers);
 
             var uniqueValidators = GetValidators(source.GetValidators(), MandatoryValidators, source.Options);
@@ -70,8 +70,7 @@ public static ImmutableConfig Create(IConfig source)
                 source.Orderer ?? DefaultOrderer.Instance,
                 source.SummaryStyle ?? SummaryStyle.Default,
                 source.Options,
-                source.BuildTimeout,
-                configAnalyse.AsReadOnly()
+                source.BuildTimeout
             );
         }
 
@@ -95,83 +94,36 @@ private static ImmutableHashSet<IDiagnoser> GetDiagnosers(IEnumerable<IDiagnoser
             return builder.ToImmutable();
         }
 
-        private static ImmutableArray<IExporter> GetExporters(IEnumerable<IExporter> exporters,
-            ImmutableHashSet<IDiagnoser> uniqueDiagnosers,
-            IList<Conclusion> configAnalyse)
+        private static ImmutableArray<IExporter> GetExporters(IEnumerable<IExporter> exporters, ImmutableHashSet<IDiagnoser> uniqueDiagnosers)
         {
-
-            void AddWarning(string message)
-            {
-                var conclusion = Conclusion.CreateWarning("Configuration", message);
-                configAnalyse.Add(conclusion);
-            }
-
-            var mergeDictionary = new Dictionary<System.Type, IExporter>();
+            var allExporters = new List<IExporter>();
 
             foreach (var exporter in exporters)
-            {
-                var exporterType = exporter.GetType();
-                if (mergeDictionary.ContainsKey(exporterType))
-                {
-                    AddWarning($"The exporter {exporterType} is already present in configuration. There may be unexpected results.");
-                }
-                mergeDictionary[exporterType] = exporter;
-            }
-
+                allExporters.Add(exporter);
 
             foreach (var diagnoser in uniqueDiagnosers)
-                foreach (var exporter in diagnoser.Exporters)
-                {
-                    var exporterType = exporter.GetType();
-                    if (mergeDictionary.ContainsKey(exporterType))
-                    {
-                        AddWarning($"The exporter {exporterType} of {diagnoser.GetType().Name} is already present in configuration. There may be unexpected results.");
-                    }
-                    mergeDictionary[exporterType] = exporter;
-                };
-
-            var result = mergeDictionary.Values.ToList();
-
+            foreach (var exporter in diagnoser.Exporters)
+                allExporters.Add(exporter);
 
             var hardwareCounterDiagnoser = uniqueDiagnosers.OfType<IHardwareCountersDiagnoser>().SingleOrDefault();
             var disassemblyDiagnoser = uniqueDiagnosers.OfType<DisassemblyDiagnoser>().SingleOrDefault();
 
             // we can use InstructionPointerExporter only when we have both IHardwareCountersDiagnoser and DisassemblyDiagnoser
-            if (hardwareCounterDiagnoser != default(IHardwareCountersDiagnoser) && disassemblyDiagnoser != default(DisassemblyDiagnoser))
-                result.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser));
+            if (hardwareCounterDiagnoser != null && disassemblyDiagnoser != null)
+                allExporters.Add(new InstructionPointerExporter(hardwareCounterDiagnoser, disassemblyDiagnoser));
 
-            for (int i = result.Count - 1; i >=0; i--)
-                if (result[i] is IExporterDependencies exporterDependencies)
+            for (int i = allExporters.Count - 1; i >= 0; i--)
+                if (allExporters[i] is IExporterDependencies exporterDependencies)
                     foreach (var dependency in exporterDependencies.Dependencies)
-                        /*
-                         *  When exporter that depends on an other already present in the configuration print warning.
-                         *
-                         *  Example:
-                         *
-                         *  // Global Current Culture separator is Semicolon;
-                         *  [CsvMeasurementsExporter(CsvSeparator.Comma)] // force use Comma
-                         *  [RPlotExporter]
-                         *  public class MyBanch
-                         *  {
-                         *
-                         *  }
-                         *
-                         *  RPlotExporter is depend from CsvMeasurementsExporter.Default
-                         *
-                         *  On active logger will by print:
-                         *  "The CsvMeasurementsExporter is already present in the configuration. There may be unexpected results of RPlotExporter.
-                         *
-                         */
-                        if (!result.Any(exporter=> exporter.GetType() == dependency.GetType()))
-                            result.Insert(i, dependency); // All the exporter dependencies should be added before the exporter
-                        else
-                        {
-                            AddWarning($"The {dependency.Name} is already present in the configuration. There may be unexpected results of {result[i].GetType().Name}.");
-                        }
-
-            result.Sort((left, right) => (left is IExporterDependencies).CompareTo(right is IExporterDependencies)); // the case when they were defined by user in wrong order ;)
-
-            return result.ToImmutableArray();
+                        allExporters.Insert(i, dependency); // All the exporter dependencies should be added before the exporter
+
+            var uniqueExporters = new List<IExporter>();
+
+            foreach (var exporter in allExporters)
+                if (!uniqueExporters.Contains(exporter, ExporterComparer.Instance))
+                    uniqueExporters.Add(exporter);
+
+            return uniqueExporters.ToImmutableArray();
         }
 
         private static ImmutableHashSet<IAnalyser> GetAnalysers(IEnumerable<IAnalyser> analysers, ImmutableHashSet<IDiagnoser> uniqueDiagnosers)
@@ -249,5 +201,14 @@ private class TypeComparer<TInterface> : IEqualityComparer<TInterface>
 
             public int GetHashCode(TInterface obj) => obj.GetType().GetHashCode();
         }
+
+        private class ExporterComparer : IEqualityComparer<IExporter>
+        {
+            public static readonly ExporterComparer Instance = new ExporterComparer();
+
+            public bool Equals(IExporter x, IExporter y) => x.GetType() == y.GetType() && x.Id == y.Id;
+
+            public int GetHashCode(IExporter obj) => obj.GetType().GetHashCode();
+        }
     }
 }

src/BenchmarkDotNet/Configs/ManualConfig.cs

@@ -20,8 +20,6 @@ namespace BenchmarkDotNet.Configs
 {
     public class ManualConfig : IConfig
     {
-        private readonly static Conclusion[] emptyConclusion = Array.Empty<Conclusion>();
-
         private readonly List<IColumnProvider> columnProviders = new List<IColumnProvider>();
         private readonly List<IExporter> exporters = new List<IExporter>();
         private readonly List<ILogger> loggers = new List<ILogger>();
@@ -54,8 +52,6 @@ public class ManualConfig : IConfig
         [PublicAPI] public SummaryStyle SummaryStyle { get; set; }
         [PublicAPI] public TimeSpan BuildTimeout { get; set; } = DefaultConfig.Instance.BuildTimeout;
 
-        public IReadOnlyList<Conclusion> ConfigAnalysisConclusion => emptyConclusion;
-
         public ManualConfig WithOption(ConfigOptions option, bool value)
         {
             Options = Options.Set(value, option);

src/BenchmarkDotNet/Exporters/AsciiDocExporter.cs

@@ -6,10 +6,10 @@ namespace BenchmarkDotNet.Exporters
 {
     public class AsciiDocExporter : ExporterBase
     {
-        protected override string FileExtension => "asciidoc";
-
         public static readonly IExporter Default = new AsciiDocExporter();
 
+        protected override string FileExtension => "asciidoc";
+
         private AsciiDocExporter()
         {
         }

src/BenchmarkDotNet/Exporters/CompositeExporter.cs

@@ -14,7 +14,7 @@ public class CompositeExporter : IExporter
 
         public CompositeExporter(ImmutableArray<IExporter> exporters) => this.exporters = exporters;
 
-        public string Name => nameof(CompositeExporter);
+        public string Id => nameof(CompositeExporter);
 
         public void ExportToLog(Summary summary, ILogger logger)
         {

src/BenchmarkDotNet/Exporters/Csv/CsvExporter.cs

@@ -6,12 +6,12 @@ namespace BenchmarkDotNet.Exporters.Csv
 {
     public class CsvExporter : ExporterBase
     {
+        public static readonly IExporter Default = new CsvExporter(CsvSeparator.CurrentCulture, SummaryStyle.Default.WithZeroMetricValuesInContent());
+
         private readonly SummaryStyle style;
         private readonly CsvSeparator separator;
         protected override string FileExtension => "csv";
 
-        public static readonly IExporter Default = new CsvExporter(CsvSeparator.CurrentCulture, SummaryStyle.Default.WithZeroMetricValuesInContent());
-
         public CsvExporter(CsvSeparator separator) : this (separator, SummaryStyle.Default.WithZeroMetricValuesInContent())
         {
         }

src/BenchmarkDotNet/Exporters/ExporterBase.cs

@@ -10,7 +10,7 @@ namespace BenchmarkDotNet.Exporters
 {
     public abstract class ExporterBase : IExporter
     {
-        public string Name => $"{GetType().Name}{FileNameSuffix}";
+        public string Id => $"{GetType().Name}{FileNameSuffix}";
 
         protected virtual string FileExtension => "txt";
         protected virtual string FileNameSuffix => string.Empty;

src/BenchmarkDotNet/Exporters/IExporter.cs

@@ -6,7 +6,7 @@ namespace BenchmarkDotNet.Exporters
 {
     public interface IExporter
     {
-        string Name { get; }
+        string Id { get; }
 
         void ExportToLog(Summary summary, ILogger logger);
         IEnumerable<string> ExportToFiles(Summary summary, ILogger consoleLogger);

src/BenchmarkDotNet/Exporters/InstructionPointerExporter.cs

@@ -31,7 +31,7 @@ internal InstructionPointerExporter(IHardwareCountersDiagnoser hardwareCountersD
             this.disassemblyDiagnoser = disassemblyDiagnoser;
         }
 
-        public string Name => nameof(InstructionPointerExporter);
+        public string Id => nameof(InstructionPointerExporter);
 
         public void ExportToLog(Summary summary, ILogger logger) { }
 

src/BenchmarkDotNet/Exporters/RPlotExporter.cs

@@ -14,7 +14,7 @@ namespace BenchmarkDotNet.Exporters
     public class RPlotExporter : IExporter, IExporterDependencies
     {
         public static readonly IExporter Default = new RPlotExporter();
-        public string Name => nameof(RPlotExporter);
+        public string Id => nameof(RPlotExporter);
 
         private const string ImageExtension = ".png";
         private static readonly object BuildScriptLock = new object();

src/BenchmarkDotNet/Running/BenchmarkConverter.cs

@@ -89,7 +89,7 @@ private static ImmutableConfig GetFullTypeConfig(Type type, IConfig config)
             var typeAttributes = type.GetCustomAttributes(true).OfType<IConfigSource>();
             var assemblyAttributes = type.Assembly.GetCustomAttributes().OfType<IConfigSource>();
 
-            foreach (var configFromAttribute in assemblyAttributes.Concat(typeAttributes))
+            foreach (var configFromAttribute in typeAttributes.Concat(assemblyAttributes))
                 config = ManualConfig.Union(config, configFromAttribute.Config);
 
             return ImmutableConfigBuilder.Create(config);

src/BenchmarkDotNet/Running/BenchmarkRunnerClean.cs

@@ -272,12 +272,6 @@ private static void PrintSummary(ILogger logger, ImmutableConfig config, Summary
             // TODO: make exporter
             ConclusionHelper.Print(logger, config.GetCompositeAnalyser().Analyse(summary).Distinct().ToList());
 
-            if (config.ConfigAnalysisConclusion.Any())
-            {
-                logger.WriteLineHeader("// * Config Issues *");
-                ConclusionHelper.Print(logger, config.ConfigAnalysisConclusion);
-            }
-
             // TODO: move to conclusions
             var columnWithLegends = summary.Table.Columns.Where(c => c.NeedToShow && !string.IsNullOrEmpty(c.OriginalColumn.Legend)).Select(c => c.OriginalColumn).ToArray();
 

src/BenchmarkDotNet/Validators/RPlotExporterValidator.cs

@@ -0,0 +1,38 @@
+using System.Collections.Generic;
+using System.Linq;
+using BenchmarkDotNet.Exporters;
+using BenchmarkDotNet.Exporters.Csv;
+
+namespace BenchmarkDotNet.Validators
+{
+    public class RPlotExporterValidator : IValidator
+    {
+        public static readonly RPlotExporterValidator FailOnError = new RPlotExporterValidator();
+
+        public bool TreatsWarningsAsErrors => true;
+
+        public IEnumerable<ValidationError> Validate(ValidationParameters validationParameters)
+        {
+            var exporters = validationParameters.Config.GetExporters();
+            if (!ContainRPlotExporter(exporters))
+                yield break;
+
+            if (!ValidateCsvExporter(exporters))
+                yield return new ValidationError(TreatsWarningsAsErrors, "RPlotExporter requires CsvMeasurementsExporter.Default. Do not override CsvMeasurementsExporter");
+        }
+
+        private static bool ContainRPlotExporter(IEnumerable<IExporter> exporters)
+        {
+            return exporters.Any(exporter => exporter.GetType() == typeof(RPlotExporter));
+        }
+
+        private static bool ValidateCsvExporter(IEnumerable<IExporter> exporters)
+        {
+            var exporter = exporters.OfType<CsvMeasurementsExporter>().FirstOrDefault();
+            if (exporter == null)
+                return false;
+
+            return exporter.Separator == CsvMeasurementsExporter.Default.Separator;
+        }
+    }
+}

tests/BenchmarkDotNet.IntegrationTests/BaselineRatioColumnTest.cs

@@ -90,7 +90,7 @@ public class TestExporter : IExporter
 
             public string Description => "For Testing Only!";
 
-            public string Name => "TestBenchmarkExporter";
+            public string Id => "TestBenchmarkExporter";
 
             public void ExportToLog(Summary summary, BenchmarkDotNet.Loggers.ILogger logger) => ExportCalled = true;