SONARPY-944 Use precomputed Typeshed symbols for third-party libraries in the Python analyzer by andrea-guarino-sonarsource · Pull Request #1061 · SonarSource/sonar-python

andrea-guarino-sonarsource · 2022-01-25T14:46:19Z

No description provided.

guillaume-dequenne

Looks good! I left some minor comments. I think it would be nice merging this in a separate branch so that I can incorporate my changes from SONARPY-945 there as well, so that we can validate every regression in sonar-security is under control before merging everything.

guillaume-dequenne · 2022-01-27T16:27:18Z

    path = os.path.join(CURRENT_PATH, relative_path)
    for root, dirs, files in os.walk(path):
        package_name = root.replace(path, "").replace("\\", ".").replace("/", ".").lstrip(".")
-        if not generate_python2_stdlib and "python2" in package_name:


Is it needed to generalize this bit? I think only the stdlib has a @python2 part in its path.

Yes Indeed I changed it to generalize. However, also third_parties has @python2 (for example six)

guillaume-dequenne · 2022-01-28T08:39:20Z

-            self.pretty_printed_name = f"Tuple[{','.join(item_names)}]"
+            if any(item.is_unknown for item in items):
+                self.kind = None
+                self.is_unknown = True


Why not have a Tuple with Any items in that case? Or just another field that mark the entirety of items as unknown?

Indeed, I hesitated a bit if creating a Tuple with Any. I took the simplest approach for the time being, if that's fine also for you, I would prefer to handle this in a separate PR. I can create a ticket for it.

guillaume-dequenne · 2022-01-28T08:45:24Z



-def save_module(ms: Union[ModuleSymbol, MergedModuleSymbol], is_debug=False, debug_dir="output"):
+def save_module(ms: Union[ModuleSymbol, MergedModuleSymbol], is_debug=False, debug_dir="output", is_stdlib=True):


I need to do the same thing for custom stubs and used a similar boolean for that. I don't think it's a good approach once we have to distinguish between the 3 cases.
Another idea I just had was to use a field on the module level to indicate its type (custom / stdlib / third party) and define the save_dir through that field. Wdyt?
In any case this doesn't need to be changed here, I can also do it in my PR when generalizing the method.

Sure, makes sense to me. I will let you do it in your PR

guillaume-dequenne · 2022-01-28T08:55:13Z

    output_dir_name = output_dir_name if python_version >= (3, 0) else f"{output_dir_name}@python2"
    opt = get_options(python_version)
-    build_result = walk_typeshed_stdlib(opt)
+    _, build_result = walk_typeshed_stdlib(opt)


If we always use build_result but not always source_paths, maybe we could change the order in which they are returned?

Sure, I can do that

Done in 75f1353

guillaume-dequenne · 2022-01-28T09:01:56Z


-  private static Map<String, Symbol> getSymbolsFromProtobufModule(String moduleName) {
-    InputStream resource = TypeShed.class.getResourceAsStream(PROTOBUF + moduleName + ".protobuf");
+  private static Map<String, Symbol> getSymbolsFromProtobufModule(String moduleName, boolean isThirdParty) {


Instead of a boolean, what about passing the directory name directly (so that this generalizes more easily with custom stubs) ?

As discussed, I will let you do that in your PR :)

guillaume-dequenne · 2022-01-28T09:06:06Z

  private static final String THIRD_PARTY_3 = "typeshed/third_party/3/";
  private static final String CUSTOM_THIRD_PARTY = "custom/";
  private static final String PROTOBUF = "protobuf/";
+  private static final String PROTOBUF_THIRD_PARTY = "protobuf/stubs/";


Currently we have stdlib under protobuf and third parties under protobuf/stubs. All of this is under types which currently holds Typeshed as well.
What about going for a protobuf_stdlib, protobuf_thid_parties and protobuf_custom all at the same level, for better clarity?

works for me! if that's fine I will let you also do that in your PR or I can do it in another separate PR

guillaume-dequenne · 2022-01-28T09:07:54Z

    resp = make_response("hello")
-    resp.headers['Access-Control-Allow-Origin'] = '*' # Noncompliant
-#   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+    resp.headers['Access-Control-Allow-Origin'] = '*' # FN


Do you know why this becomes a FN? Is this the one related to the other ticket you created?

This is because attributes were missing from typeshed classes. I think this change is not needed after I rebase on master :)

I confirm, after rebase the issue is raised again :)

guillaume-dequenne · 2022-01-28T09:13:38Z


 def main():
    save_merged_symbols()
-    save_merged_symbols(is_third_parties=True)


I guess this change is not needed.

Fixed in 90ef511

…s in the Python analyzer

sonarsource-next · 2022-01-28T10:28:47Z

SonarQube Quality Gate

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

98.3% Coverage
0.0% Duplication

guillaume-dequenne

LGTM

GitOrigin-RevId: c7c8ff06ab0e3f046cf936c4656b4185c1012161

andrea-guarino-sonarsource changed the title ~~Sonarpy 944~~ SONARPY-944 Use precomputed Typeshed symbols for third-party libraries in the Python analyzer Jan 25, 2022

andrea-guarino-sonarsource force-pushed the SONARPY-944 branch 6 times, most recently from d421c7e to 89990eb Compare January 27, 2022 13:53

andrea-guarino-sonarsource marked this pull request as ready for review January 27, 2022 14:32

andrea-guarino-sonarsource requested a review from guillaume-dequenne January 27, 2022 15:07

guillaume-dequenne requested changes Jan 28, 2022

View reviewed changes

andreaguarino added 5 commits January 28, 2022 11:13

SONARPY-944 Use precomputed Typeshed symbols for third-party librarie…

75f1353

…s in the Python analyzer

Add protobuf files for stubs

5d5306b

Use protobuf stubs for third parties

369dd7f

typeshed_serializer should serialize correctly aliased symbols

90ef511

Update protobuf files with aliased symbols

e46e978

andrea-guarino-sonarsource force-pushed the SONARPY-944 branch from 89990eb to e46e978 Compare January 28, 2022 10:16

andrea-guarino-sonarsource changed the base branch from master to typeshed-rework January 28, 2022 10:19

andrea-guarino-sonarsource requested a review from guillaume-dequenne January 28, 2022 10:42

guillaume-dequenne approved these changes Jan 28, 2022

View reviewed changes

andrea-guarino-sonarsource merged commit 5588096 into typeshed-rework Jan 28, 2022

andrea-guarino-sonarsource deleted the SONARPY-944 branch January 28, 2022 12:49

hashicorp-vault-sonar-prod Bot pushed a commit that referenced this pull request Apr 23, 2026

SONARPY-4050 Fix Cobertura source path resolving (#1061)

c2d1910

GitOrigin-RevId: c7c8ff06ab0e3f046cf936c4656b4185c1012161



		def save_module(ms: Union[ModuleSymbol, MergedModuleSymbol], is_debug=False, debug_dir="output"):
		def save_module(ms: Union[ModuleSymbol, MergedModuleSymbol], is_debug=False, debug_dir="output", is_stdlib=True):

Conversation

andrea-guarino-sonarsource commented Jan 25, 2022

Uh oh!

guillaume-dequenne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarsource-next Bot commented Jan 28, 2022

Uh oh!

guillaume-dequenne left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants