miss_hit icon indicating copy to clipboard operation
miss_hit copied to clipboard

flag plattform specific directory separators

Open jand271 opened this issue 2 years ago • 8 comments

What kind of feature is this?

  • New feature in MISS_HIT

Your MATLAB/Octave environment

  • MATLAB
  • r2022a

MISS_HIT component affected Choose one or more of the below:

  • Code metrics

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.

jand271 avatar Aug 02 '22 19:08 jand271

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.

Remi-Gau avatar Aug 09 '22 08:08 Remi-Gau

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.

florianschanda avatar Aug 09 '22 12:08 florianschanda

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')

Remi-Gau avatar Aug 09 '22 13:08 Remi-Gau

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 avatar Aug 09 '22 14:08 florianschanda

@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.

florianschanda avatar Aug 13 '22 12:08 florianschanda

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

jand271 avatar Aug 15 '22 18:08 jand271

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.

florianschanda avatar Aug 20 '22 11:08 florianschanda

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

RodneyRichardson avatar Feb 23 '24 15:02 RodneyRichardson