unused_import: false rule violation (importing SwiftUI to use ObservableObject and @Published in a macOS project)

grantneufeld commented 1 year ago

New Issue Checklist

[x] Updated SwiftLint to the latest version
[x] I searched for existing GitHub issues

Describe the bug

Multi-target (iOS and macOS) project, when building for macOS (platform is important for my example: the code is a replacement for the EditMode SwiftUI provides on iOS but not on macOS), I have a file that imports SwiftUI to be able to specify that a class conforms to the ObservableObject protocol, and has a @Published property.

The unused_import rule is incorrectly reporting that import call as unused. If I remove the import line, I get errors in Xcode:

Cannot find type 'ObservableObject' in scope
Unknown attribute 'Published'

So, clearly, the compiler uses the import.

Complete output when running SwiftLint, including the stack trace and command used

Xcode Build:

$ xcodebuild -project WebExtractor.xcodeproj -scheme WebExtractor -destination platform=macOS,arch=x86_64 > xcodebuild.log

Outputs:

2023-08-10 20:06:15.685 xcodebuild[73360:7531983] DVTCoreDeviceEnabledState: DVTCoreDeviceEnabledState_Disabled set via user default (DVTEnableCoreDevice=disabled)

The SwiftLint Analyse:

$ swiftlint analyze --compiler-log-path xcodebuild.log

Outputs:

Analyzing Swift files in current working directory
... (many Collecting lines omitted) ...
... (many Analyzing lines omitted) ...
Analyzing 'EditMode.swift' (9/14)
/Users/user/Projects/MyApp/MyApp/Views/EditMode.swift:1:1: warning: Unused Import Violation: All imported modules should be required to make the file compile (unused_import)
... (more Analyzing lines omitted) ...
Done analyzing! Found 1 violation, 0 serious in 41 files.

Environment

SwiftLint version 0.52.4
Installation method used: Homebrew
Paste your configuration file:

# Here’s the whole thing, even though just the analyzer_rules: unused_import matters for this report
disabled_rules:
  - for_where
  - force_cast
opt_in_rules:
  - accessibility_label_for_image
  - accessibility_trait_for_button
  - anonymous_argument_in_multiline_closure
  - array_init
  - attributes
  - balanced_xctest_lifecycle
  - closure_body_length
  - closure_end_indentation
  - closure_spacing
  - collection_alignment
  - comma_inheritance
  - conditional_returns_on_newline
  - contains_over_filter_count
  - contains_over_filter_is_empty
  - contains_over_first_not_nil
  - contains_over_range_nil_comparison
  - convenience_type
  - direct_return
  - discarded_notification_center_observer
  - discouraged_assert
  - discouraged_none_name
  - discouraged_object_literal
  - discouraged_optional_boolean
  - discouraged_optional_collection
  - empty_collection_literal
  - empty_count
  - empty_string
  - empty_xctest_method
  - enum_case_associated_values_count
  - expiring_todo
  - explicit_init
  - fallthrough
  - fatal_error_message
  - file_header
  - file_name
  - file_name_no_space
  - file_types_order
  - first_where
  - flatmap_over_map_reduce
  - function_default_parameter_at_end
  - ibinspectable_in_extension
  - identical_operands
  - implicit_return
  - implicitly_unwrapped_optional
  - indentation_width
  - joined_default_parameter
  - last_where
  - legacy_multiple
  - legacy_objc_type
  - legacy_random
  - let_var_whitespace
  - literal_expression_end_indentation
  - local_doc_comment
  - lower_acl_than_parent
  - missing_docs
  - modifier_order
  - multiline_arguments
  - multiline_arguments_brackets
  - multiline_function_chains
  - multiline_literal_brackets
  - multiline_parameters
  - multiline_parameters_brackets
  - nimble_operator
  - no_extension_access_modifier
  - no_magic_numbers
  - nslocalizedstring_key
  - nslocalizedstring_require_bundle
  - number_separator
  - object_literal
  - operator_usage_whitespace
  - optional_enum_case_matching
  - overridden_super_call
  - override_in_extension
  - pattern_matching_keywords
  - period_spacing
  - prefer_self_in_static_references
  - prefer_self_type_over_type_of_self
  - prefer_zero_over_explicit_init
  - private_action
  - private_subject
  - prohibited_super_call
  - quick_discouraged_call
  - quick_discouraged_focused_test
  - quick_discouraged_pending_test
  - raw_value_for_camel_cased_codable_enum
  - reduce_into
  - redundant_nil_coalescing
  - redundant_type_annotation
  - required_enum_case
  - return_value_from_void_function
  - self_binding
  - shorthand_optional_binding
  - single_test_class
  - sorted_enum_cases
  - sorted_first_last
  - sorted_imports
  - static_operator
  - strict_fileprivate
  - strong_iboutlet
  - superfluous_else
  - switch_case_on_newline
  - test_case_accessibility
  - toggle_bool
  - trailing_closure
  - unavailable_function
  - unhandled_throwing_task
  - unneeded_parentheses_in_closure_argument
  - unowned_variable_capture
  - untyped_error_in_catch
  - vertical_parameter_alignment_on_call
  - vertical_whitespace_between_cases
  - vertical_whitespace_closing_braces
  - vertical_whitespace_opening_braces
  - weak_delegate
  - xct_specific_matcher
  - yoda_condition

analyzer_rules:
  - capture_variable
  - explicit_self
  - typesafe_array_init
  - unused_import

attributes:
  always_on_same_line:
    - "@Environment"
    - "@IBAction"
    - "@NSManaged"
identifier_name:
  excluded:
    - id
    - to
type_name:
  excluded:
    - ID

Xcode 14.3.1
macOS 13.5 (Intel)
Do you have a sample that shows the issue?

// EditMode.swift
import SwiftUI

final class EditMode: ObservableObject {
    @Published var isEditing = false
}

(I normally have the content of that file wrapped in #if os(macOS) … #endif compiler directives, but trimmed that out and built just for the macOS destination to see if that would change anything, but the incorrect rule warning came out the same.)

grantneufeld commented 1 year ago

Update:

If the import SwiftUI line is removed from the sample failing file above, two errors show up in Xcode:

// removed:
// import SwiftUI

/// Since EditMode only exists for iOS, here’s a simplified version for macOS.
final class EditMode: ObservableObject { // 🛑 Cannot find type 'ObservableObject' in scope
    /// True if the view is in “edit” mode.
    @Published var isEditing = false // 🛑 Unknown attribute 'Published'
}

If I instead swap in Foundation for SwiftUI, then the errors go away, and SwiftLint Analyze does not incorrectly report an unused_import violation.

Weird.

Even though import SwiftUI makes the code work, the rule is considered violated when I use that import instead of import Foundation. 🤷

grantneufeld commented 1 year ago

More data for this:

import Foundation
enum A11y {
    static let min: CGFloat = 44
}

The above example triggers the false rule violation. (CGFloat is not an available type unless defined by an import).

Substituting import CoreGraphics for the import Foundation line also produces the false rule violation.

But substituting import CoreFoundation.CFNumber does not trigger the rule violation.

So it seems that the unused_import rule is triggering anytime the import is needed, but could be replaced with a more specific import. Weird.

PaulTaykalo commented 3 months ago

Right, so what is happening is this:

swiftlint is asking for a cursorInfo for ObservableObject token
sourcekit replies that ObservableObject is actually Combine.ObservableObject from Combine module.
swiftlint then decides that no objects were references from SwiftUI and removes it from imports

The issue is that ObservableObject is actually was imported throught SwiftUI.

So suggestion is to have something like allowed_transitive rules, which will check if we actually using one of the objects and inside the re-exported module.

https://github.com/realm/SwiftLint/blob/b42f6ffe77159aed1060bf607212a0410c7623b8/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/UnusedImportConfiguration.swift#L38

OR:

As an alternative, we could use the same configuration for detecting such issues, and not remove imports that are actually used like Umbrella import :)

https://github.com/realm/SwiftLint/pull/5622 as a draft

@jpsim @SimplyDanny

realm / SwiftLint