Log globbing anomalies: max path, drive enumeration

[cdmihai]

• Drive enumeration. The more general problem is when the glob pattern contains properties which evaluate to empty. ($(src)/**/*.cs evaluates to /**/*.cs).
• Original wildcard pattern is retuned (as it sometimes happens). Maybe also include the reason.

[rainersigwald]

Let's spike on this in the next sprint: if it's easy to do (because we have a logging context where we'd like to collect the timing/bad patterns and log them) we'll do it, and if not then we'll describe what does exist here.

[Forgind]

I have two partial solutions (one per commit) here. Do you think it's better to try to sense a globbing anomaly while expanding a property or wait until we're globbing and assume no one wants to enumerate their whole drive? I could also try to pass information about empty properties from one to the other, but that would be complicated and a lot of extra information, so I don't think it's worth it.

[cdmihai]

Keep in mind these things (property evaluation, glob parsing/expanding) are on the hot path, so newly added logic (like additional regex matching) has a good chance of being picked up by RPS. For globbing anomalies I would just change the globbing code to record diagnostics and bubble them upwards to the evaluator / expander which can log appropriate message. For drive enumeration I would avoid duplicating the knowledge of glob syntax between the glob parsing code and the expander (glob parsing code should be the only one that knows the syntax). The glob parsing code should just record a "drive enumeration" diagnostic when it detects that the fixed directory part is a drive, which then is bubbled up to a layer appropriate for logging.

[rokonec]

@ladipro has any of your recent changes in globs addressed that?

[ladipro]

@rokonec no, the recent globbing changes did not improve logging. The unintended drive enumeration problem was somewhat alleviated in #5669 but I agree that this issue should be in backlog.

[mruxmohan4]

@ladipro are there any updates on logging/surfacing an exception for max path during wildcard expansion?

[ladipro]

@mruxmohan-msft there have been some internal discussions but no conclusion yet. The issue still exists. I'll leave a comment in #7029.

[rokonec]

@JanKrivanek This might be good candidate for analyzers V1, especially max path, it is so serious that it maybe shall be warning but warnings could break some builds