976353 - Install Eclipse formatting and code clean up defaults for AndroidEclipse backend generated projects

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nick Alexander :nalexander [he/him]]

Let's make new developers lives easier. We can iterate on these settings as needed.

Comment 1

[Gregory Szorc [:gps]]

BenWa has a template in bug 973770 for formatting. I think you should it.

Comment 2

[Gregory Szorc [:gps]]

Should share it. goto fail.

Comment 3

[Nick Alexander :nalexander [he/him]]

(In reply to Gregory Szorc [:gps] from comment #2)
> Should share it. goto fail.

Could do; BenWa's formatting is for the Eclipse CDT plugin, and this is for the JDT plugin.

Sharing such formatting between projects means writing things to the user's Eclipse workspace, and I have avoided doing so because it's quite tricky to arrange, and because it's a clear demarcation between what the backend owns and what the user owns.

Comment 4

[Nick Alexander :nalexander [he/him]]

Attachment: Part 1: Make AndroidEclipse backend install formatting defaults. r=rnewman

The most important part here is 4 spaces, no tabs.  We can iterate on
other things over time.

The installation is "per project", when it could in theory be shared,
because it's quite difficult to actually arrange the sharing.  We'd need
to write into an Eclipse workspace (which we don't currently even know).

Comment 5

[Nick Alexander :nalexander [he/him]]

Attachment: Part 2: Make AndroidEclipse backend install clean up defaults. r=rnewman

This installs:

* Remove unused imports
* Add missing '@Override' annotations
* Add missing '@Override' annotations to implementations of interface methods
* Add missing '@Deprecated' annotations
* Remove unnecessary casts
* Remove unnecessary '$NON-NLS$' tags
* Organize imports
* Remove trailing white spaces on all lines

The installation is "per project", when it could in theory be shared,
because it's quite difficult to actually arrange the sharing.  We'd need
to write into an Eclipse workspace (which we don't currently even know).

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 8381031 [details] [diff] [review]
Part 1: Make AndroidEclipse backend install formatting defaults. r=rnewman

Review of attachment 8381031 [details] [diff] [review]:
-----------------------------------------------------------------

rs

::: python/mozbuild/mozbuild/backend/templates/android_eclipse/.settings/org.eclipse.jdt.core.prefs
@@ +89,5 @@
> +org.eclipse.jdt.core.formatter.indent_statements_compare_to_block=true
> +org.eclipse.jdt.core.formatter.indent_statements_compare_to_body=true
> +org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_cases=true
> +org.eclipse.jdt.core.formatter.indent_switchstatements_compare_to_switch=false
> +org.eclipse.jdt.core.formatter.indentation.size=2

4?

Comment 7

[Richard Newman [:rnewman]]

Comment on attachment 8381033 [details] [diff] [review]
Part 2: Make AndroidEclipse backend install clean up defaults. r=rnewman

Review of attachment 8381033 [details] [diff] [review]:
-----------------------------------------------------------------

rs

::: python/mozbuild/mozbuild/backend/templates/android_eclipse/.settings/org.eclipse.jdt.ui.prefs
@@ +1,2 @@
>  #filter substitution
> +cleanup.add_default_serial_version_id=true

Recommend not.

@@ +9,5 @@
> +cleanup.add_missing_override_annotations_interface_methods=true
> +cleanup.add_serial_version_id=false
> +cleanup.always_use_blocks=true
> +cleanup.always_use_parentheses_in_expressions=false
> +cleanup.always_use_this_for_non_static_field_access=false

Oh, how I want this.

@@ +28,5 @@
> +cleanup.qualify_static_member_accesses_through_instances_with_declaring_class=true
> +cleanup.qualify_static_member_accesses_through_subtypes_with_declaring_class=true
> +cleanup.qualify_static_member_accesses_with_declaring_class=false
> +cleanup.qualify_static_method_accesses_with_declaring_class=false
> +cleanup.remove_private_constructors=true

?

@@ +32,5 @@
> +cleanup.remove_private_constructors=true
> +cleanup.remove_trailing_whitespaces=true
> +cleanup.remove_trailing_whitespaces_all=true
> +cleanup.remove_trailing_whitespaces_ignore_empty=false
> +cleanup.remove_unnecessary_casts=true

This seems worrisome.

Comment 8

[Nick Alexander :nalexander [he/him]]

https://hg.mozilla.org/integration/fx-team/rev/09240240ab18
https://hg.mozilla.org/integration/fx-team/rev/88a8926eb7cd

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/88a8926eb7cd
https://hg.mozilla.org/mozilla-central/rev/09240240ab18