florianschanda / miss_hit

MATLAB Independent, Small & Safe, High Integrity Tools - code formatter and more
GNU General Public License v3.0
163 stars 21 forks source link

flag plattform specific directory separators #260

Open jand271 opened 2 years ago

jand271 commented 2 years ago

What kind of feature is this?

Your MATLAB/Octave environment

MISS_HIT component affected Choose one or more of the below:

Describe the solution you'd like I'm working at a company with code repo among many developers that needs to work on windows and linux. The developers keep specifying path construction functions manually (e.g., 'example/path' instead of using fullfile). This is driving me crazy and causing bugs everywhere.

It would be nice if miss_hit would say "I found a '/' or '\'. Use fullfile instead".

This would force engineers to properly specify paths.

Remi-Gau commented 2 years ago

As much as I would like to have this type of feature, my hunch is that this would be hard to implement but I may be wrong.

Will let @florianschanda give his take on this.

florianschanda commented 2 years ago

First, I apologise for the delay in reply.

Second, I am not sure I understand fully what you're asking for. Are you trying to avoid \ appearing in parameters to functions such as cd?

So for example, is this what you want?

cd C:\potato
     ^ don't do this you dirty windows user you

If so, then this is hard/fragile. First, I need to understand which functions are path manipulating, and second I need to identify which of them actually are and which ones were overloaded/overwritten by the user (which I cannot do yet).

Then even IF semantic name resolution works, then what about this:

random_var = "C:\potato"
cd(random_var);

I will close this ticket as won't do under that assumption. If I got that assumption wrong, please re-open with a comment.

Remi-Gau commented 2 years ago

Will let @jand271 correct me I am wrong but I assumed he meant doing things like this:

this_dir = pwd();
my_file = [this_dir, '/', 'foo',  '/',  'bar', '/',  'my_file.txt']

where as the following is preferable

this_dir = pwd();
my_file = fullfile(this_dir,  'foo',   'bar',  'my_file.txt')
florianschanda commented 2 years ago

Yeah, that's even worse than what I imagined :D

MISS_HIT is actually really dumb still. It is 100% syntactic, there is no semantic resolution or interpretation done. You can get really far on that basis (as is evident) but stuff like this is (currently) beyond its capabilities.

florianschanda commented 2 years ago

@jand271 for reference, if you could submit a few examples here; just so I have something on record and can add the testsuite... that would be appreciated. Because if I do get sem done; then this check could be implementable.

jand271 commented 2 years ago

Yes, @Remi-Gau, that is exactly what I'm talking about. Dealing with nightmare at work with this.

I am definitely not familiar with how difficult it is to create tools like MISS_HIT or MISS_HIT's internals, but perhaps MISS_HIT can catch all / and \ within "" (to filter out / and \ in comments) and require fullfile.

This regex expression catches them in my code base...but idk how to get regex to highlight all of the \/ (as apposed to a single one...but I guess it is good enough).

\'*[//,\\]*\'

Examples:

gunzip([target_dir '\' serverName]);
[obsDir 'mgex-obs/'];
pathNameFormat = [settings.dir '/%d/%03d/'];
path = [basedir 'nav-daily/'];

If MISS_HIT doesn't add this...then I might need to make a custom checker in bash before the code kills someone lol

florianschanda commented 2 years ago

OK, I will re-open as I think we can do something in MH Lint maybe. It won't be perfect, but it might catch a few of these.

RodneyRichardson commented 7 months ago

I believe / is a valid cross-platform path separator. This would only need to flag \ as a potential cross-platform issue.