I don’t understand many of the aspects of your refactor of the warning system.
I think that it reduces the flexibility of the warning system.

First, it doesn’t make much sense to have Manager implement DeprecationSupervisor. That would be like making the Entity class have a method called swimSpeed. SwimSpeed doesn’t apply to most entities. Any manager that cares about deprecation should extend the interface itself, and the interface should have its own getter for the list of supervisors.

Second, I disagree with the concept of a DeprecationSupervisor itself. Should it really be the ExecutorManager’s responsibility to run the logic necessary to pick out a warning from a token? If you were to change the Token names, should executorManager ever be in the changed files? I think not. This logic belongs in the lexer. If you want to separate the functionality into its own method, just make a method in the lexer itself.

Finally, I don’t agree with the changes you’ve made to DeprecationWarning. The new name implies that it means “something, somewhere is deprecated”, which simply doesn’t fit the name because the message is linked to what Token you pass in. If we wanted to use it to warn about something such as a deprecated config option, would we now have a token called CONFIG_OPTION? Once again, Token has crossed the boundaries of the scope it’s meant to be used for. It should be removed from the constructor. Also, warnings were meant to be specific to every situation, and nothing more than an object-oriented template for a String, so it’s unlikely that a single subclass of warning can apply to more than one scenario. DeprecationWarning could probably be used for placeholders as well, but it wouldn’t be specific enough for much else. It should be renamed accordingly.

Hey, sorry for the change without notice

The key idea is that decoupling and testability.

The previous implementation used static method of AbstractExecutorManager so that it causes unnecessary direct connection between Parser and the interface.

I want to keep the core.skript package isolated from the rest of interfaces to keep the project cohesive, and it eventually provide testability of the code like I did in the TestParser.java

Like you said, we can do something like Listener system of Bukkit API so that only few classes with the DeprecayionSupervisor will be used, but it will need quite a effort, and I don't find it worth the time to implement such system.

For DeprecationWarnijg, I was thinking about the possibility where Placeholder or any other tokens that may be deprecated in the future. It would be super easy to add that this way since all we need to do is check on supervisors to see if the token is deprecated. But also. It is true that we can just hard code everything in the Parser or Lexer.

Anyway, key idea is less coupling and more cohesion. Feel free to modify as long as avoid coupling and strive testability

You haven’t solved the coupling problem, only hidden in while creating a host of new problems.
Now Manager has to import and extend DeprecationSupervisor.
But right now, only one class actually uses it
Yet all classes extend it.
This is clearly an instance of the Refused Bequest anti-pattern
If you are really that much against the Parser importing the manager, why not do it the other way? The executor manager will calle a static setter in the parser to set the list of deprecated executors, problem solved!

Another problem is that now, the ExecutorManager is responsible for doing the work that the parser should be doing.
It’s like grabbing a random parser method and sticking it in some other class. Sound far-fetched? That’s exactly what these changed did. The client managers have to worry about logic involving tokens and the parse tree, which should be solely the parser’s concern. If the implementation of tokens changed, the executor manager would need to change as well. This is VERY broken encapsulation, in fact, the Token class should be package-private anyway.

These changes also reduce the flexibility of the warnings system. You may think generalizing the warning to work with any kind of token is a smart move, but you’re forgetting that errors need to be user-friendly, and most of the token names aren’t. Additionally, it can’t represent anything beyond a single token, which doesn’t apply to a lot of warning types. Representing several types of warnings with the same class also removes the possibility of filtering the warnings by class. Most importantly, it contains a token, breaking encapsulation (as I said earlier)

It's just my perspective that the core.script package should be independent of other packages. I am aware of that the children classes do not necessarily have to implement DeprecationSupervisor. That's why I talked about the Listener interface, which the Bukkit API manually look for methods annotated with @EventHandler. But it would be too expensive to implement such a system just for small functionality.

Though, let's imagine that the code wasn't refactored; if I were to move only the core.script package into a different project, then it would be not easy since the Parser is actually depending on the ExecutorManager. Or even in the future, if we keep using the same pattern, it will add even more dependencies (PlaceholderManager for example) outside the Parser class. But with the current implementation, if I don't inject the DeprecationSupervisor, the Parser will work even without any of those classes. So it's completely isolated from these classes.

Actually your idea is good too, that we just call the static setter to add deprecated tokens.

As I said, the change is almost a temporary measure, so you are welcome to add anything as long as you are concerned with coupling problems. (I invited you as a collaborator for a reason). One suggestion would be hard code the DeprecationSupervisor in the Parser itself, so it does not have to depend on anything but Parser itself. It also makes sense since deprecating a token is the job of Parser itself, and the ExecutorManager doesn't have anything to do with it.

In fact, I just used this pattern to keep the codes you have written as much as possible. So that's where complication started :) Just we need a little more in-depth communication.

And yeah, I have to admit that the DeprecationWarning is not user-friendly indeed. I just changed for my personal favor. Feel free to modify as that was my mistake.

We can still have a DeprecationManager, but it will simply delegate its calls to the appropriate classes instead of being extended in any way. We also don't need some sort of listener inside it, all we need is

public static List<String> getDeprecatedExecutors() {
    return AbstractExecutorManager.getDeprecatedExecutors();
}

Or the static setter

I like the first idea better since it avoids the deprecation mechanics being openly mutable, but it also introduces a class that does nothing, which is a bit of a downside.

Let's make it injected by static method.

It works for both our ideas.

@wysohn
This is well tested and pretty much done
Should we close this?

I'm planning on redesigning the entire system with you, not just start coding right away, but to start from writing UML diagram and then to actual implementation, so we can combine things altogether.

I recently found you have another check-up routine inside the Lexer too, so we will need to talk about that.

I'm thinking of a more generalized system (not just specific to $ sign) so that it can adapt any cases without modification of the class itself. Using Matcher for each line can be one possibility.

But for now, we can close it as I do not have much time atm.