richterger / Perl-LanguageServer

Language Server for Perl
Other
219 stars 53 forks source link

use taint on syntax check #172

Closed wielandp closed 1 year ago

wielandp commented 1 year ago

Like described in issue https://github.com/richterger/Perl-LanguageServer/issues/143 Using taint check for syntax check to prevent error message when first line is like "#!/usr/bin/perl -T" or "#!/usr/bin/perl -t"

richterger commented 1 year ago

This add's the -T taint check always, that might break other code that does not use tainting. I think taint checks should be an option.

wielandp commented 1 year ago

That is what I thought first too. But in this case I use it only with -c for syntax checking. Maybe you are right, because some code is executed when using -c. Not sure, begin blocks? I can't really imagine code, that would break, but who knows.

The option version didn't look nice to me, since it should detect automatically and no need for adding options.

The third way to do it, would be scan the first line and apply -T if it is in the hash bang line.

Let me know, how to solve it

richterger commented 1 year ago

Actually also perl -c can execute code. For example all BEGIN blocks will run, also when you run perl -c . So I think -T should only be applied if necessary. I would go with your third idea, scan the first line and if there is -T in it, also apply it to perl -c.

Also it would be a good idea the the -T option also for the debugger. Here it would make sense to do automatic scaning of line 1 and in addition have an option to set -T independed of line 1

wielandp commented 1 year ago

I go for the small solution (just option, no auto detect)

wielandp commented 1 year ago

If I want to debug code with hashbang and taint, I have the same problem. Shall I rename the option from "Use -T for syntax check." to ""Use -T for syntax check and debugging."? Or shall I add a new config option for debugging? I will write a pull request for a new config option. So the setting option is only for syntax check

richterger commented 1 year ago

Haveing a similar option in lauch.json for debugging, look like a very good idea.