Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Plugin support #1739

Merged
merged 63 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
7abd760
Removed internal
MikaelMayer Jan 20, 2022
daa54ee
Fixed error
MikaelMayer Jan 20, 2022
867015b
Fixed all dependencies errors
MikaelMayer Jan 20, 2022
54218fc
Initial support for /compiler: or -compiler: flag
MikaelMayer Jan 20, 2022
8951357
Correct way to load an external DLL
MikaelMayer Jan 20, 2022
90d8255
Better comments
MikaelMayer Jan 20, 2022
39eca00
Added a test for the /compiler: option
MikaelMayer Jan 20, 2022
4b8db03
Support for compiler options in DafnyLanguageServer
MikaelMayer Jan 20, 2022
3d9553c
Plugin for everything instead of just resolution.
MikaelMayer Jan 21, 2022
3664526
Fixed multiple loading of the option
MikaelMayer Jan 21, 2022
4539424
Better error reporting
MikaelMayer Jan 21, 2022
27bd135
Removed useless code.
MikaelMayer Jan 21, 2022
2b87a90
More debugging code (revert this commit)
MikaelMayer Jan 21, 2022
98b6d97
Revert "More debugging code (revert this commit)"
MikaelMayer Jan 21, 2022
93a170c
WIP: adding a new test
MikaelMayer Jan 25, 2022
7a71322
Merge branch 'master' into resolving-plugin-support
MikaelMayer Jan 25, 2022
31578e8
Fixed the test of DafnyLanguageServer to prove plugins work.
MikaelMayer Jan 25, 2022
3b82af7
Crystal clear API documentation for Resolver plugins
MikaelMayer Jan 25, 2022
ed40974
Added missing two newlines
MikaelMayer Jan 25, 2022
d40790b
Fixed XUnit tests
MikaelMayer Jan 26, 2022
b3d9683
Better names
MikaelMayer Jan 26, 2022
2516be6
Proper Configuration for plugins to extend
MikaelMayer Jan 26, 2022
0b104e4
Updated README
MikaelMayer Jan 26, 2022
b99acbd
Updated documentation
MikaelMayer Jan 26, 2022
1c8084c
Merge branch 'master' into resolving-plugin-support
MikaelMayer Jan 26, 2022
9b405de
Ensure it works even without Configuration.
MikaelMayer Jan 27, 2022
62ad0ed
support for commas in arguments.
MikaelMayer Jan 27, 2022
52821b3
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Jan 27, 2022
647b233
Plugin test should use the module's first positional token.
MikaelMayer Jan 27, 2022
7698e7c
* Ensure even the default module's default token is not empty even if…
MikaelMayer Jan 27, 2022
eeb264b
PostResolve also for the entire program.
MikaelMayer Jan 27, 2022
d5b653f
Addressed review comments
MikaelMayer Jan 27, 2022
6e236bd
Fixed typos
MikaelMayer Jan 27, 2022
9f9e3be
Addressed review comments
MikaelMayer Jan 31, 2022
e8ddf7c
Reviews implemented.
MikaelMayer Feb 1, 2022
0f3ee09
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 1, 2022
a27836e
Fixed the synchronicity problem between two different plugin tests.
MikaelMayer Feb 2, 2022
4e38b80
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 2, 2022
40b6a35
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 2, 2022
b09f5d5
Separated Rewriter from plugins from internal rewriters.
MikaelMayer Feb 3, 2022
e4c58e5
Rewriter
MikaelMayer Feb 3, 2022
6becf88
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 3, 2022
57f7589
Note about the experimental feature of plugins
MikaelMayer Feb 3, 2022
4e52ba6
Cleanup plugin + moved initialization earlier so that it's run only o…
MikaelMayer Feb 4, 2022
f9c3a17
Better syntax
MikaelMayer Feb 4, 2022
1d34931
Property again
MikaelMayer Feb 4, 2022
8f8a135
Extracted a method to load the plugins
MikaelMayer Feb 4, 2022
3a4d7ab
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 4, 2022
749c3f0
Display plugin loading errors as part of the diagnostics.
MikaelMayer Feb 4, 2022
6151452
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 4, 2022
c42d10c
Adressed review comments
MikaelMayer Feb 7, 2022
15855d3
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 7, 2022
b6fcbd8
Plugins as separate CS files
MikaelMayer Feb 8, 2022
1fd20d4
Removed unused static variable
MikaelMayer Feb 8, 2022
6bdcd98
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 8, 2022
7a5c48b
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 8, 2022
e17738b
plugin as a record
MikaelMayer Feb 8, 2022
64fc6a2
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 8, 2022
21188aa
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 8, 2022
568f1a5
Implemented review comments
MikaelMayer Feb 9, 2022
2463c10
Merge branch 'resolving-plugin-support' of https://github.com/dafny-l…
MikaelMayer Feb 9, 2022
eb35472
Refactored a factory + made static variable readonly
MikaelMayer Feb 10, 2022
223c9f8
Merge branch 'master' into resolving-plugin-support
MikaelMayer Feb 10, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Source/Dafny/AST/DafnyAst.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ public string Name {
}
}
}

public IToken GetFirstTopLevelToken() {
return DefaultModuleDef.GetFirstTopLevelToken();
}
}

public class Include : IComparable {
Expand Down Expand Up @@ -4223,6 +4227,17 @@ public bool IsEssentiallyEmptyModuleBody() {
}
return true;
}

public IToken GetFirstTopLevelToken() {
return TopLevelDecls.OfType<ClassDecl>()
.SelectMany(classDecl => classDecl.Members)
.Where(member => member.tok.line > 0)
.Select(member => member.tok)
.Concat(TopLevelDecls.OfType<LiteralModuleDecl>()
.Select(moduleDecl => moduleDecl.ModuleDef.GetFirstTopLevelToken())
.Where(tok => tok.line > 0)
).FirstOrDefault(Token.NoToken);
}
}

public class DefaultModuleDecl : ModuleDefinition {
Expand Down
47 changes: 46 additions & 1 deletion Source/Dafny/DafnyOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
using System.Text;
using System.Diagnostics.Contracts;
using System.IO;
using System.Reflection;
using System.Text.RegularExpressions;
using JetBrains.Annotations;
using Microsoft.Dafny.Plugins;
using Bpl = Microsoft.Boogie;

namespace Microsoft.Dafny {
Expand Down Expand Up @@ -135,12 +138,13 @@ public enum IncludesModes {
// Working around the fact that xmlFilename is private
public string BoogieXmlFilename = null;

public List<Plugin> Plugins = new();

public virtual TestGenerationOptions TestGenOptions =>
testGenOptions ??= new TestGenerationOptions();

protected override bool ParseOption(string name, Bpl.CommandLineOptionEngine.CommandLineParseState ps) {
var args = ps.args; // convenient synonym

switch (name) {
case "dprelude":
if (ps.ConfirmArgumentCount(1)) {
Expand Down Expand Up @@ -230,6 +234,41 @@ protected override bool ParseOption(string name, Bpl.CommandLineOptionEngine.Com
return true;
}

case "Plugin":
case "plugin": {
if (ps.ConfirmArgumentCount(1)) {
var pluginAndArgument = args[ps.i];
if (pluginAndArgument.Length > 0 &&
pluginAndArgument[0] == '"' &&
pluginAndArgument[^1] == '"'
) {
var unescapeRegex = new Regex(@"\\""|\\\\");
pluginAndArgument = unescapeRegex.Replace(pluginAndArgument.Substring(1, pluginAndArgument.Length - 2),
match => match.Groups[0].Value == @"\""" ? "\"" : @"\"
);
}
if (pluginAndArgument.Length > 0) {
var pluginArray = pluginAndArgument.Split(',');
var pluginPath = pluginArray[0];
var arguments = Array.Empty<string>();
if (pluginArray.Length >= 2) {
// There are no commas in paths, but there can be in arguments
var argumentsString = string.Join(',', pluginArray.Skip(1));
// Parse arguments, accepting and remove double quotes that isolate long arguments
var splitter = new Regex(@"""((?:[^""]|\\"")*)""|([^ ]+)");
arguments = splitter.Matches(argumentsString).Select(
matchResult => matchResult.Groups[1].Success ?
matchResult.Groups[1].Value.Replace(@"\""", @"""") :
matchResult.Groups[2].Value
).ToArray();
}
Plugins.Add(new Plugin(pluginPath, arguments, errorReporter));
}
}

return true;
}

case "Main":
case "main": {
if (ps.ConfirmArgumentCount(1)) {
Expand Down Expand Up @@ -806,6 +845,12 @@ verification outcome
Note that the C++ backend has various limitations (see Docs/Compilation/Cpp.md).
This includes lack of support for BigIntegers (aka int), most higher order
functions, and advanced features like traits or co-inductive types.
/plugin:<path to one assembly>[ <arguments>]
(experimental) One path to an assembly that contains at least one
instantiatable class extending Microsoft.Dafny.Plugin.Rewriter.
It can also extend Microsoft.Dafny.Plugin.Configuration to receive arguments
More information about what plugins do and how define them:
https://github.com/dafny-lang/dafny/blob/master/Source/DafnyLanguageServer/README.md#about-plugins
/Main:<name>
The (fully-qualified) name of the method to use as the executable entry point.
Default is the method with the {{:main}} atrribute, or else the method named 'Main'.
Expand Down
82 changes: 82 additions & 0 deletions Source/Dafny/Plugin.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Reflection;
using Microsoft.Dafny.Plugins;

namespace Microsoft.Dafny;

/// <summary>
/// This class is internal to Dafny. It wraps an assembly and an extracted configuration from this assembly,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is internal to Dafny.

What do you mean by that? Why not mark the visibility as internal then?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed "is internal to Dafny. It wraps" since most classes in Dafny are never used outside of Dafny but could be in the future.

/// The configuration provides the methods to parse command-line arguments and obtain Rewriters
/// </summary>
public class Plugin {
public Configuration Configuration;

private readonly string path;
MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
private readonly Assembly assembly;
private System.Type[] rewriterTypes;

class AutomaticConfiguration : Configuration {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a record instead of a class. Why is this a nested class?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a nested class so that it's under the name Plugin.AutomaticConfiguration, which is in the same semantic domain than Plugins.Configuration which is the base class that it extends. Nothing else than the class Plugin needs this class which is very short, so it makes sense to keep it as a nested class.

Based on your suggestion, I considered using a record instead of a class, but as it is extending an abstract class, as far as I know, this is not possible.

Copy link
Member

@keyboardDrummer keyboardDrummer Feb 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing else than the class Plugin needs this class which is very short, so it makes sense to keep it as a nested class.

Okay. In my experience with C# codebases, nested classes are only used when access to private members of the containing class is needed, but that's not the case here. For a class that's used by another class through public members, I'm used to using top-level classes, which compared to nested classes may separate concerns more and reduce indentation. I don't have any hard arguments for this way of using nested classes, so feel free to keep it like it is, but consider this an extra data point ^,^

Based on your suggestion, I considered using a record instead of a class, but as it is extending an abstract class, as far as I know, this is not possible.

You'd have to change the abstract class into an abstract record

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Scala, you'd put this class in the companion object name that has the same name as of the class itself, so that it stays in the same namespace.
In other languages like Java, you can use the word "static" to emphasize that the sub-class is not depending on an instance of the surrounding class.

I read a bit about records, and it seems they are classes that have publicly visible constructor variables (it's nice to avoid the constructor idiom, but it exposes private variables outside of the class), and they implement all the methods necessary for being considered as first-class value (value equality, hashing, toString).
Here, an AutomaticPluginConfigurationhas for only private field an array of Func<ErrorReporter, Rewriter>, which does not support value equality. Moreover, we can't put the keyword "static" in front of a record.
I did implement your suggestion because it makes the code more legible, but I'm still unsure of that step.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to revert the use of "abstract record" for PluginConfiguration back to an "abstract class", after testing its usability.
Since the goal is for developers to extend the class PluginConfiguration, if I mark it as an abstract record, they will be forced to implement internal methods such as PluginConfiguration.<Clone>$(), and the other goodies that record routinely offer, that abstract record actually require. It would be fine if they developed their plugins in c#, but the dotnet ecosystem has many other languages which do not know about records (e.g. F#).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, nice UX research :)

private Func<ErrorReporter, Rewriter>[] rewriters;
public AutomaticConfiguration(Func<ErrorReporter, Rewriter>[] rewriters) {
this.rewriters = rewriters;
}

public override Rewriter[] GetRewriters(ErrorReporter errorReporter) {
return rewriters.Select(funcErrorReporterRewriter =>
funcErrorReporterRewriter(errorReporter)).ToArray();
}
}

public static IEnumerable<System.Type> GetConfigurationsTypes(Assembly assembly) {
return assembly.GetTypes()
.Where(t => t.IsAssignableTo(typeof(Configuration)));
}

public Plugin(string pluginPath, string[] args, ErrorReporter reporter) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you put the constructor immediately below the fields?

MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
path = pluginPath;
assembly = Assembly.LoadFrom(path);
rewriterTypes = CheckPluginForRewriters(assembly, path);
Configuration = LoadConfiguration(assembly, args, path, rewriterTypes);
}

private static System.Type[] CheckPluginForRewriters(Assembly assembly, string path) {
System.Type[] rewriterTpes = assembly.GetTypes().Where(t =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sake of consistency within the same class (single letter identifiers), I'd use type instead of t as the identifier name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it necessary to specify System.Type[] explicitly here?

t.IsAssignableTo(typeof(Rewriter))).ToArray();
// Checks about the plugin to be well-behaved.
if (!rewriterTpes.Any()) {
throw new Exception($"Plugin {path} does not contain any Microsoft.Dafny.Plugins.Rewriter");
MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
}

return rewriterTpes;
}

private static Configuration LoadConfiguration(Assembly assembly, string[] args, string path, System.Type[] rewriterTypes) {
Configuration configuration = null;
foreach (var configurationType in GetConfigurationsTypes(assembly)) {
if (configuration != null) {
throw new Exception(
$"Only one class should extend Microsoft.Dafny.Plugins.Configuration from {path}. Please remove {configurationType}.");
MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
}

configuration = (Configuration)Activator.CreateInstance(configurationType);

if (configuration == null) {
throw new Exception($"Could not instantiate a {configurationType} from {path}");
}

configuration.ParseArguments(args);
}

configuration ??= new AutomaticConfiguration(
rewriterTypes.Select<System.Type, Func<ErrorReporter, Rewriter>>((System.Type rewriterType) =>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the type arguments to Select can be left out.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to remove it several times (because the IDE suggests it to me), but every time I do, dotnet compiler says it needs that type.

Copy link
Member

@camrein camrein Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The explicitly typed lambda expressions should be redundant due to the type arguments to Select.
However, the requirement for the explicit type arguments might be a good sign that it's best to extract the projection into a separate method in terms of readability:

    pluginConfiguration ??= new AutomaticPluginConfiguration {
      Rewriters = rewriterTypes.Select(CreateRewriterFactory).ToArray()
    };
    return pluginConfiguration;
  }

  private static Func<ErrorReporter, Rewriter> CreateRewriterFactory(System.Type rewriterType) {
    return errorReporter => (Rewriter)Activator.CreateInstance(rewriterType, new object[] { errorReporter });
  }

I leave it up to you if you want to apply this change.

(ErrorReporter errorReporter) =>
Activator.CreateInstance(rewriterType, new object[] { errorReporter }) as Rewriter).ToArray());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use (Rewriter)Activator.CreateInstance(rewriterType, new object[] { errorReporter }) instead. Alternatively, the LINQ statement .Cast<Rewriter>() might be handy too.

return configuration;
}

public IEnumerable<IRewriter> GetRewriters(ErrorReporter reporter) {
return Configuration.GetRewriters(reporter).Select(rewriter => new PluginRewriter(reporter, rewriter));
}
}
31 changes: 31 additions & 0 deletions Source/Dafny/Plugins/Configuration.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using System;
using System.Collections.Generic;

namespace Microsoft.Dafny.Plugins;

/// <summary>
/// Plugins should define a class that extends Configuration,
/// in order to receive plugin-specific command-line arguments by overwriting the method `ParseArguments`
/// It is also used to provide to Dafny a list of Rewriter using the method `GetRewriters`
///
/// If the plugin defines no Configuration, then Dafny will instantiate every sub-class
/// of Rewriter from the plugin, providing them with an ErrorReporter in the constructor
/// as the first and only argument.
/// </summary>
public abstract class Configuration {
MikaelMayer marked this conversation as resolved.
Show resolved Hide resolved
/// <summary>
/// A Microsoft.Dafny.Plugins.Configuration will be automatically instantiated an arguments
/// will be provided to the plugin by the method `ParseArguments``;
robin-aws marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="args">The arguments passed to the plugin</param>
public virtual void ParseArguments(string[] args) {
}

/// <summary>
/// Override this method to provide specific rewriters to Dafny
/// </summary>
/// <returns>a list of Rewriter that are going to be used in the resolution pipeline</returns>
public virtual Rewriter[] GetRewriters(ErrorReporter errorReporter) {
return Array.Empty<Rewriter>();
}
}
55 changes: 55 additions & 0 deletions Source/Dafny/Plugins/Rewriter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
using System.Diagnostics.Contracts;

namespace Microsoft.Dafny.Plugins {
/// <summary>
/// A class that plugins should extend, in order to provide an extra Rewriter to the pipeline.
///
/// If the plugin defines no Configuration, then Dafny will instantiate every sub-class
/// of Rewriter from the plugin, providing them with an ErrorReporter in the constructor
/// as the first and only argument.
/// </summary>
public abstract class Rewriter {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the effort in making the contract as clear as possible here. I think this meets the bar for the feature as long as its marked as experimental. I do think we'll need more of a mechanism to break the build if we change the various AST types in a way that break plugins before we fully commit, though, so good to start thinking about that now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For that, we need access relevant plugins that we intend to support, indeed. This will be the case for compilers soon.

Copy link
Member

@robin-aws robin-aws Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, that's not the right model. We need to explicitly decide on exactly what parts of the implementation plugins have access to, and at the very least document it clearly. Even better, we should use mechanisms to make sure we don't expose things we don't intend to keep the same in the future (i.e. public vs internal), and don't accidentally change things we said we wouldn't (i.e. API compatibility tests, which there may be tooling out there for on .NET).

Even with all that, there will still be unfortunate cases where users start relying on something you didn't intend them to (I refer you to the infamous XKCD on that subject: https://xkcd.com/1172/ :) ) and changing things without breaking clients will sometimes be tricky. If you make your API "whatever the users we're aware of are using", though, you're making it that much harder to scale your resources and far more likely you're going to break people and/or tying your hands for refactoring internals for the better in the future.

Copy link
Member

@robin-aws robin-aws Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

</rant> :)

Note I'm not asking for any deeper changes on this PR right now - it will be great to have folks experiment as long as they know the API isn't finalized yet!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I totally agree on that !

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for plugins that aim to support custom compiler back-ends to override anything other than the final PostResolve(Program program) ?

I feel the use-case of this PR is underspecified. The title reads "This PR creates an extensible plugin structure, which is primarily used to provide Dafny with extra Rewriters", which I think is more of a how than a why.

My impression from our conversations is that this PR is meant to allow custom back-ends to add additional error reporting. I would think that this only requires adding errors after resolution has completed entirely. Is that right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a need for plugins that aim to support custom compiler back-ends to override anything other than the final PostResolve(Program program) ?

Plugin support is not about just compiler back-ends (see last answer).

I think is more of a how than a why.

I added a statement after the how to explain why.

My impression from our conversations is that this PR is meant to allow custom back-ends to add additional error reporting. I would think that this only requires adding errors after resolution has completed entirely. Is that right?

This PR goes beyond our conversation indeed. It opens the door to plugins actually rewriting the AST before it is resolved (see PreResolve()), and with little effort to later support custom back-ends later.
Hence it's not just "custom back-ends", since we could definitely have plugins that do other checks that are not part of any back-end. For example, a plugin that verifies some naming conventions (think of a linter), another that extern methods are tested, another plugin performs some tests on external methods,, another plugin that replaces some macros before resolution... support custom back-ends.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New API looks good :)

/// <summary>
/// Used to report errors and warnings, with positional information.
/// </summary>
protected readonly ErrorReporter Reporter;

/// <summary>
/// Constructor that accepts an ErrorReporter
/// You can obtain an ErrorReporter two following ways:
/// * Extend a Configuration class, and override the method GetRewriters(), whose first argument is an ErrorReporter
/// * Have no Configuration class, and an ErrorReporter will be provided to your class's constructor.
///
/// Then you can use the protected field "reporter" like the following:
///
/// reporter.Error(MessageSource.Compiler, token, "[Your plugin] Your error message here");
///
/// The token is usually obtained on expressions and statements in the field `tok`
/// If you do not have access to them, use moduleDefinition.GetFirstTopLevelToken()
/// </summary>
/// <param name="reporter">The error reporter. Usually outputs automatically to IDE or command-line</param>
public Rewriter(ErrorReporter reporter) {
Contract.Requires(reporter != null);
Reporter = reporter;
}
/// <summary>
/// Override this method to obtain a module definition after the entire resolution pipeline
/// You can then report errors using reporter.Error (see above)
/// </summary>
/// <param name="moduleDefinition">A module definition after it
/// is resolved, type-checked and SCC/Cyclicity/Recursivity have been performed</param>
public virtual void PostResolve(ModuleDefinition moduleDefinition) {
Contract.Requires(moduleDefinition != null);
}

/// <summary>
/// Override this method to obtain the final program after the entire resolution pipeline
/// after the individual PostResolve on every module
/// You can then report errors using reporter.Error (see above)
/// </summary>
/// <param name="program">The entire program after it is fully resolved</param>
public virtual void PostResolve(Program program) {
Contract.Requires(program != null);
}
}
}
Loading