Skip to content

Cleanup of warnings / possible not checked nullpointers #1610

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 20, 2020

Conversation

saschaarthur
Copy link
Contributor

Description

Exception handling / code cleanup

Does your commit message include the wording below to reference a specific issue in this repo?

There was plenty of warnings by my IDE (IntelliJ).
Most of the stuff was because of failing null pointer checks and unreachable/unused code / as well not needed function calls.

Logic is 100% the same.

Does your pull request have unit tests?

Everything fixed here should be covered by already existing unit tests

@cla-bot
Copy link

cla-bot bot commented May 11, 2020

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign the CLA at https://www.nativescript.org/cla.
CLA has not been signed by users: @saschaarthur.
After signing the CLA, you can ask me to recheck this PR by posting @cla-bot check as a comment to the PR.

@saschaarthur
Copy link
Contributor Author

@cla-bot check

@cla-bot cla-bot bot added the cla: yes label May 11, 2020
@cla-bot
Copy link

cla-bot bot commented May 11, 2020

The cla-bot has been summoned, and re-checked this pull request!

if (!isKotlinClass) {
return new JavaClassDescriptor(javaClass);
}
return new JavaClassDescriptor(javaClass);

This comment was marked as abuse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if statement seems unnecessary as we return some lines above in the case of Kotlin originating bytecode and the only case we will reach this point is when we're dealing with Java originating bytecode

Copy link
Contributor Author

@saschaarthur saschaarthur May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IDE complained that the if statement is always false.
Which is correct, its never set to true and then reached.

This comment was marked as abuse.

@@ -61,7 +62,7 @@ private static void writeInt(int value, StreamWriter out) throws Exception {
private static void writeMethodInfo(MethodInfo mi,
HashMap<String, Integer> uniqueStrings, StreamWriter outValueStream)
throws Exception {
int pos = uniqueStrings.get(mi.name).intValue();

This comment was marked as abuse.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be autounboxed by the JVM, and this is also a build time step. I believe this change is ok.

if (!isKotlinClass) {
return new JavaClassDescriptor(javaClass);
}
return new JavaClassDescriptor(javaClass);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The if statement seems unnecessary as we return some lines above in the case of Kotlin originating bytecode and the only case we will reach this point is when we're dealing with Java originating bytecode

@@ -61,7 +62,7 @@ private static void writeInt(int value, StreamWriter out) throws Exception {
private static void writeMethodInfo(MethodInfo mi,
HashMap<String, Integer> uniqueStrings, StreamWriter outValueStream)
throws Exception {
int pos = uniqueStrings.get(mi.name).intValue();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be autounboxed by the JVM, and this is also a build time step. I believe this change is ok.

@saschaarthur saschaarthur force-pushed the fix/warning_cleanup branch from ff0c7ec to c2c7932 Compare May 11, 2020 22:27
@saschaarthur
Copy link
Contributor Author

readded .intValue() unboxing, to be very clear that we want to have here int not Integer

@vmutafov
Copy link
Contributor

@saschaarthur there is no difference in this case. If the Integer value is null it would crash in both ways. Calling intValue() is not necessary like you first suggested.

@vmutafov vmutafov merged commit fa31074 into NativeScript:master May 20, 2020
vmutafov added a commit that referenced this pull request May 26, 2020
* chore:Update CHANGELOG for v6.5.0

* Make buildMetadata aware of whitelist/blacklist files (#1590)

* - warning cleanups (#1610)

* Bump to 6.5.1

* Update CHANGELOG.md for 6.5.1

* Hardcode first emulator

* Use wait-for-device

Co-authored-by: Martin Bektchiev <[email protected]>
Co-authored-by: saschaarthur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants