search

facebook / infer

A static analyzer for Java, C, C++, and Objective-C
http://fbinfer.com/
MIT License
14.9k stars 2.01k forks source link

CSharp Pulse String Models #1611

Closed matjin closed 2 years ago

matjin commented 2 years ago

Adds pulse models for IsNullOrWhiteSpace/IsNullOrEmpty

facebook-github-bot commented 2 years ago

@jvillard has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

jvillard commented 2 years ago

Would it be possible to run make fmt ahead of submitting pull requests so they have a chance of passing make test?

jvillard commented 2 years ago

This looks good! But the models are not used unless you plug them into the main Pulse matcher here:

https://github.com/facebook/infer/blob/23a50725929447a47d712633c91772b5f0bc2534/infer/src/pulse/PulseModels.ml#L14-L18

matjin commented 2 years ago

Would it be possible to run make fmt ahead of submitting pull requests so they have a chance of passing make test?

I tried this -- but I just get this as output followed by a hang --

root@39d772db2b50:/app/fork/infer# sudo make fmt parallel ocamlformat -i ::: $(git diff --name-only --diff-filter=ACMRU $(git merge-base origin/main HEAD) | grep ".mli\?$") perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LANG = "en_US.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). perl: warning: Setting locale failed. perl: warning: Please check that your locale settings: LANGUAGE = (unset), LC_ALL = (unset), LANG = "en_US.UTF-8" are supported and installed on your system. perl: warning: Falling back to the standard locale ("C"). parallel: Warning: Input is read from the terminal. You either know what you parallel: Warning: are doing (in which case: YOU ARE AWESOME!) or you forgot parallel: Warning: ::: or :::: or to pipe data into parallel. If so parallel: Warning: consider going through the tutorial: man parallel_tutorial parallel: Warning: Press CTRL-D to exit. ^Cmake: *** [Makefile:301: fmt] Error 255

facebook-github-bot commented 2 years ago

@matjin has updated the pull request. You must reimport the pull request before landing.

facebook-github-bot commented 2 years ago

@matjin has updated the pull request. You must reimport the pull request before landing.

matjin commented 2 years ago

This looks good! But the models are not used unless you plug them into the main Pulse matcher here:

https://github.com/facebook/infer/blob/23a50725929447a47d712633c91772b5f0bc2534/infer/src/pulse/PulseModels.ml#L14-L18

Thanks! Done.

jvillard commented 2 years ago

I tried this -- but I just get this as output followed by a hang --

I wonder if the locale problem mentioned in the message is causing perl to parse the command-line arguments incorrectly or if something else is going on... What about LANG=C sudo make fmt?

matjin commented 2 years ago

I tried this -- but I just get this as output followed by a hang --

I wonder if the locale problem mentioned in the message is causing perl to parse the command-line arguments incorrectly or if something else is going on... What about LANG=C sudo make fmt?

Now get the below with a hang:

root@39d772db2b50:/app/fork/infer# LANG=C sudo make fmt parallel ocamlformat -i ::: $(git diff --name-only --diff-filter=ACMRU $(git merge-base origin/main HEAD) | grep ".mli\?$") parallel: Warning: Input is read from the terminal. You either know what you parallel: Warning: are doing (in which case: YOU ARE AWESOME!) or you forgot parallel: Warning: ::: or :::: or to pipe data into parallel. If so parallel: Warning: consider going through the tutorial: man parallel_tutorial parallel: Warning: Press CTRL-D to exit.

I'm working on a fairly minimal Ubuntu Docker which may not have some of the expected dependencies (i.e. I manually added vim/ocamlformat) -- is there anything else I might need?

jvillard commented 2 years ago

Re: make fmt: I think I know what's going on: the error message says you forgot ::: which is clearly ridiculous as it's right here. What it means to say is that there's nothing after the :::: I get the same error if I run parallel ocamlformat -i :::. That means the git commands that follow (git diff --name-only --diff-filter=ACMRU $(git merge-base origin/main HEAD) | grep ".mli?$") don't return anything. Are you running make fmt outside of a git repo by any chance, or is your main branch not at origin/main? In any case, why these commands return nothing is the thing to debug.

Does make fmt_all work for you? You could use it as a workaround, it's just slower. I (and others) use ocamlformat from within the IDE so it auto-formats either on save or on some key binding, it could be useful too.

matjin commented 2 years ago

Needed to update the .ocamlformat file -- looked like it had the wrong version. After I followed the instructions --

"Setting up your project to use the default profile and the OCamlFormat version you installed (hopefully the last one) in this .ocamlformat file is considered good practice:

profile = default version = 0.21.0 "

Looks like I have it working for next time :)

jvillard commented 2 years ago

I'm confused, there's already a .ocamlformat in the infer repo and it's not yet at version 0.21.0. You can get the correct ocamlformat version for infer with make devsetup, or perhaps make devsetup OPAM_PIN_OCAMLFORMAT=yes.

matjin commented 2 years ago

Ah -- that does the trick. Probably I needed to run make devsetup from the start. Now it looks like everything is working as expected.