feat: tempfile without flush or close #101
Conversation
| title: Warning for tempfile without flush | ||
| --- | ||
|
|
||
| "Be cautious when using `$F.name` without preceding it with `.flush()` or `.close()`, as it may result in an error. This is because the file referenced by `$F.name` might not exist at the time of use. To prevent issues, ensure that you either call `.flush()` to write any buffered data to the file or close the file with .close() before referencing `$F.name`. |
There was a problem hiding this comment.
Don't just copy and paste quotes from the docs. At the very least the leading quote mark isn't helpful and confuses this.
| @@ -0,0 +1,227 @@ | |||
| --- | |||
| title: Warning for tempfile without flush | |||
There was a problem hiding this comment.
Use a shorter file name. And don't call it a "warning" - the level of a diagnostic is decided by the consumer: https://docs.grit.io/guides/config#patterns
| $fileOpen <: not contains `$f.flush()`, | ||
| $fileOpen <: not contains `$f.close()`, | ||
| $fileOpen <: contains `$f.name`, | ||
| $fileOpen => `# BAD: should add $f.flush() or $f.close() before calling $f.name \n $fileOpen` |
There was a problem hiding this comment.
Don't manually add annotations like this via rewrites. Grit already has support for checking things - if no meaningful fix is available, you can skip having a rewrite at all.
In this case, a meaningful autofix might be inserting f.close() on the line before the f.name() call.
There was a problem hiding this comment.
Hi @morgante
In this case, a meaningful autofix might be inserting f.close() on the line before the f.name() call.
Yes, true I am trying to build the same.
any suggestion, if to build this,
cmd = [binary_name, fout.name, *[str(path) for path in targets]]
parsing this and add $f.close() before this, become difficult
There was a problem hiding this comment.
You don't need to handle all cases. It's fine to skip any that are too complex.
For inserting things it's often best to find the containing statement() and append to that.
iamnamananand996
changed the title
|
Hi @morgante, this PR ready for review. |
Be cautious when using
$F.namewithout preceding it with.flush()or.close(), as it may result in an error. This is because the file referenced by$F.namemight not exist at the time of use. To prevent issues, ensure that you either call.flush()to write any buffered data to the file or close the file with .close() before referencing$F.name.grit studio link: https://app.grit.io/studio?key=yeST0wXjbRx38MojeG0jn