Implement original source verification

[aldesantis]

This PR implements original source verification. It closes #10.

Behavior and implementation

The verification behavior is very similar to Deface's :original option.

Basically, we compute a combined hash of all the files where a given module is defined. The hash is stored in a prepender for that module. If any of the original source locations are updated, the hash changes and causes Prependers to throw an error, asking the user to make sure their prepender is still relevant.

There are a few aspects of the implementation I am still not sure about:

• I am still not entirely sure this approach is a good idea. People should already be testing their prependers with automated tests. With that said, I also imagine there are a few situations that are not covered by tests.
• It would be helpful for the user to have a list of the files that have changed in the original source, but this would also require storing the hash for each file, making the configuration of the prepender potentially very long.

There are also a couple additional features we may implement:

• We could add the ability for users to enforce original source verification on all prependers.
• We could add the ability for users to set the behavior for outdated prependers. The current behavior is raising an exception, but users may want to just print a warning instead.

While implementing the feature, I had to radically change the Prependers API. This shouldn't be a problem since we're still in 0.x. Instead of just relying on the Prependers::Prepender module doing everything, I introduced the concept of annotations, i.e. modules that extend the functionality of Prependers. Original source verification was implemented as the Verify annotation, and the :namespace option was turned into the Namespace annotation.

[aldesantis]

@kennyadsl @mtylty @elia curious to hear your thoughts here!

[kennyadsl]

I like this! Being optional this will only impact people wanting to use it.

Not sure it's possibile with the new implementation but this could not be breaking if we deprecate calling Prependers::Prepender.new without a "special" argument that we use when calling it from inside acts_as_prepender. This way people using the old version can gradually transition to the new one. Do you think it's doable?

[aldesantis]

@kennyadsl I think that could work, I'll give it a try in a few.

[elia]

Looks great! I left a few comments and a suggestion for a simpler API when the macro method is not used. 👏

I would also like to suggest an alternative name for "acts_as_prepender" (which sounds a bit dated to me) but I don't have a good counter-proposal so just take it as some useless feedback 😅

[elia]

any reason for not using define_singleton_method here? I mean performance or otherwise…

[aldesantis]

define_singleton_method forces me to assign the namespace to a temporary variable due to scoping issues, which I didn't really like:

_namespace = @namespace
base.define_singleton_method(:__prependers_namespace__) { _namespace }

I feel like a plain class_eval is much more straightforward. However, if there are any specific reasons for preferring define_singleton_method, please let me know!

[aldesantis]

@kennyadsl @elia this should be ready for a final review, thanks for the feedback!

[mtylty]

I like the idea but I don't think this is needed, we're adding complexity and making the gem harder to understand.

I'm also a bit biased because I've always seen developers struggling completely with Deface's original option: it looks like nobody cared/understood why that's important.

I think we should work on providing better tools for testing what you're prepending instead of adding some better workarounds.

[aldesantis]

@mtylty I see your point — tests are definitely the way to go for testing most decorators. However, I think there are some cases that are not covered by tests: for instance, what if the feature you are implementing or the bug you are fixing in your prepender is fixed in the upstream? In that case, you should remove your prepender entirely, and a test won't be able to tell you that.

The feature itself is quite simple (it's just 60 lines of code, all the other changes in this PR are not specific to the verification) and I think it could be a useful additional tool in our suite, but I also think it should be used with caution and not be treated as a substitute for writing tests.

[kennyadsl]

Works for me!