CommandHelper

CommandHelper

46.5k Downloads

autoconcat not removed for empty auto_includes

PseudoKnight opened this issue ยท 5 comments

commented

When executing an auto_include that has no code, it incorrectly encounters a leftover autoconcat function whose exec method throws an error. This even produces the error on clean installations, since the primary auto_include.ms only has commented out code. It's unaffected by strict mode or SA settings.

commented

Thinking about it, this is very likely not specific to auto includes, but rather to empty .ms files in general. That would make a test for this quite trivial (just see whether an empty string and a file with only some comments compiles). Note that just attempting to compile an empty string won't yield the same results, as the compiler takes a shortcut for those.

commented

Oh, I think the error changed. Looking back in discord logs there was a "An error occurred while analyzing included file" with an unknown source issue with empty auto_includes I found on March 31st. If so, this would be at least the third time empty auto_includes caused an error. We should add a unit test.

commented

That issue is unrelated and solved in c24bbfe on April first. But adding a unit test would be nice indeed.

commented

@LadyCailin Can you take a look at this? When I replaced this:
https://github.com/EngineHub/CommandHelper/blame/918bd125fa45c69ff30dfa072daef94a5212fd1c/src/main/java/com/laytonsmith/core/MethodScriptCompiler.java#L2120-L2122
with

if(!root.hasChildren()
		&& !(root.getData() instanceof CFunction && root.getData().val().equals(__autoconcat__.NAME))) {
	return;
}

the __autoconcat__ was rewritten as intended, but one of the new @closure() unit tests breaks with stacktrace:

Caused by: java.lang.Error: Native class class com.laytonsmith.core.constructs.IVariable does not provide a typeof annotation
        at com.laytonsmith.core.FullyQualifiedClassName.forNativeClass(FullyQualifiedClassName.java:109)
        at com.laytonsmith.core.constructs.CClassType.get(CClassType.java:116)
        at com.laytonsmith.core.constructs.Construct.typeof(Construct.java:502)
        at com.laytonsmith.core.constructs.Construct.typeof(Construct.java:492)
        at com.laytonsmith.core.constructs.InstanceofUtil.isInstanceof(InstanceofUtil.java:78)
        at com.laytonsmith.core.constructs.InstanceofUtil.isInstanceof(InstanceofUtil.java:124)
        at com.laytonsmith.core.functions.StringHandling$sconcat.optimizeDynamic(StringHandling.java:259)
        at com.laytonsmith.core.MethodScriptCompiler.optimize(MethodScriptCompiler.java:2576)
        ... 36 more

I think that you are somewhere relying on __autoconcat__ not being rewritten, where you instead should probably be checking for sconcat() or __statements__() based on the strict mode state. Right now, the implementation of MethodScriptComputer.rewriteAutoconcats() is not rewriting some __autoconcat__ nodes, which is in violation of its documentation.

Note that this is a severe issue that prevents users from using CH on their servers if they have one or more ms files where everything is commented out.