1708462 - Warn on new usages of OS.File, suggest using IOUtils instead

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, enhancement)

Description

[Marco Castelluccio [:marco]]

We are getting rid of OS.File usage during startup (bug 986145). Over time, we might want to get rid of OS.File completely in favor of IOUtils.
We could introduce a new linter that warns about new usages of OS.File and suggests using IOUtils instead.

Comment 1

[:Gijs (he/him)]

I know a little bit about linters - is there an example of a linter that examines diffs and only complains about new instances? Or do we just write a linter and add exceptions for all the known cases or something?

Comment 2

[Marco Castelluccio [:marco]]

(In reply to :Gijs (he/him) from comment #1)

I know a little bit about linters - is there an example of a linter that examines diffs and only complains about new instances? Or do we just write a linter and add exceptions for all the known cases or something?

We don't have linters that examine diffs AFAIK, we rely on the review bot for that.

Two options:

• write a linter and don't add exceptions for the known cases. Since the level will be "warning", developers won't be forced to fix the issues found by it. "mach lint --outgoing` will only analyze touched files anyway, so the noise will be relatively low in this case;
• write a linter and add exceptions for the known cases.

Comment 3

[Marco Castelluccio [:marco]]

Gijs, still interested?

Comment 4

[:Gijs (he/him)]

(In reply to Marco Castelluccio [:marco] from comment #3)

Gijs, still interested?

Yes. Unfortunately, it would appear that there are so many uses still in the codebase (over 1000) that it's probably not yet the right time to add a linter for this. Barret, what do you think?

Comment 5

[Beth Rennie [:beth] (she/her)]

I think adding a linter set to warning could be a good idea for now. Then when we start burning down the remaining OS.File usage, we can swap it over to a hard error.

Comment 6

[:Gijs (he/him)]

Attachment: Bug 1708462 - add warning level linter for OS.File use, r?Standard8

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3d7d04fa5cc4
add warning level linter for OS.File use, r=Standard8

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/3d7d04fa5cc4