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

It should not be necessary to call VerifyICSharpCodeDecompiler.Enable #400

Closed
tom-englert opened this issue Dec 18, 2022 · 6 comments
Closed

Comments

@tom-englert
Copy link
Contributor

tom-englert commented Dec 18, 2022

it should just work out of the box.

@tom-englert tom-englert changed the title It should not be necessary to call VerifyICSharpCodeDecompiler.Enable, it should just work out of the box. It should not be necessary to call VerifyICSharpCodeDecompiler.Enable Dec 18, 2022
@SimonCropp
Copy link
Member

this is problematic for a couple of reasons

  • it is desirable to be able to Verify ICSharpCode directly. ie i would prefer to have to remember to call VerifyICSharpCodeDecompiler.Enable once ather than remember that i always need to pass in a custom type that exists in Verify.ICSharpCode.Decompiler just to get the side effect of a hidden VerifyICSharpCodeDecompiler.Enable.
  • there is an ecosystem of extensions that all use the "enable in module init" pattern

@tom-englert
Copy link
Contributor Author

Actually I don't see why this is a problem.

  • When you pass in a custom type to Verify (e.g. await Verify(new TypeToDisassemble(file, type))), it requires that this custom type is registered.
  • If the type is not registered, there is no warning or error, it's just not working at all, which is very annoying.

So why not ensure that if you use such a custom type, it simply ensures that it's registered with Verify.

It does not break any other use case, just makes it more seamless to use. You can still can call Enable by your own, if you like, and everything else will work the same, no matter if the types are registered or not.

Since this pattern is used in other extensions, too, maybe there should be a more global solution, like e.g. in Serilog where they look for such registration methods via reflection in all loaded assemblies?

@SimonCropp
Copy link
Member

but await Verify(new AssemblyToDisassemble(file)) requires people to always remember to use AssemblyToDisassemble every single time they verify an assembly. ie this wont work

using var file = new PEFile(assembly2Path);
await Verify(file);

@tom-englert
Copy link
Contributor Author

So we make every days life harder, just to optimize one very rare edge case?

@SimonCropp
Copy link
Member

"passing the actual type from the underlying library" is not a rare edge case. it is how the majority of verification happens

@SimonCropp
Copy link
Member

currently undocumented, but u can call VerifierSettings.InitializePlugins(); which will do a dll scan and call initialize on all verify plugins

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants