search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.96k stars 21.15k forks source link

Github CodeQL Support #30524

Open Anutrix opened 5 years ago

Anutrix commented 5 years ago

Godot version:

Any

OS/device including version:

Any

Issue description:

https://lgtm.com/projects/g/godotengine/godot/overview/ Shows some errors and warnings. It would be good if Godot had LGTM alert count icon too with AppVeyer, Travis, Triage and Weblate in README. Ready to close this issue if it seems useless, irrelevant, duplicate or previously discussed.

Steps to reproduce: Visit the links, checkout the errors and warnings.

Minimal reproduction project:

Anutrix commented 5 years ago

Some of LGTM warnings/errors are:

  1. Change gmtime and localtime to gmtime_r and localtime_r in os_unix.cpp(Info).
  2. Change data type of width_cache and wrap_amount_cache from int to unsigned int(Info).
  3. Use a different variable name or assign to global variable frame_delay at https://github.com/godotengine/godot/blob/master/main/main.cpp#L2003 (because it currently hides a global variable).
  4. At https://github.com/godotengine/godot/blob/master/editor/plugins/editor_preview_plugins.cpp#L548, c is always >= 33 because we earlier checked for c>32. Hence, c>='!' or 33 is always true and c == '\t' is always false. So, they can be removed. Update:
  5. Fix a lot(over 50) of missing header guards(https://lgtm.com/projects/g/godotengine/godot/alerts/?mode=list&id=cpp%2Fmissing-header-guard) as it might help header guard dependent compiler optimizations (related discussion at https://github.com/godotengine/godot/issues/18143).
Anutrix commented 4 years ago

Can the maintainers install the LGTM GitHub app for Godot?

Anutrix commented 2 years ago

lgtm.com is going down in Dec 2022(https://github.blog/2022-08-15-the-next-step-for-lgtm-com-github-code-scanning/) so the badge for it in our readme.md and the page https://lgtm.com/projects/g/godotengine/godot/alerts?mode=list will stop working.

To make it continue to work after that, we need to enable it on Github itself for this repo.

Requesting maintainers to discuss this when they have time and close this ticket if it isn't worth the effort.

Anutrix commented 1 year ago

Since LGTM.com has shutdown after being acquired by Github, the badge hasn't been implemented by Github yet. However, the static checks from Semmle/LGTM seem to be in Github's CodeQL and are useful. I've renamed the ticket as there's interest on this: https://github.com/godotengine/godot/pull/75948. Maybe we can close this once that is merged.